Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: #2863 - onboarding black tooltip now bottom positioned #2889

Merged
merged 3 commits into from
Sep 6, 2022

Conversation

monsieurtanuki
Copy link
Contributor

What

  • During the onboarding, there's a black tooltip.
  • It was vertically centered (regardless of the body), which does not make sense on small devices.
  • Now the tooltip is at the bottom, on top of the bottom action bar
    • it's visible
    • it's in a place that the user is expected to see (because of the "previous" / "next" buttons)
    • we avoid the previous confusion displaying a de-facto non clickable "next" button (if you saw the tooltip and the "next" button, a click on the "next" button actually meant a click on the tooltip)

Screenshot

Capture d’écran 2022-09-01 à 10 24 35

Fixes bug(s)

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2022

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 6.94%. Comparing base (54eae2b) to head (010fc11).
Report is 1613 commits behind head on develop.

Files with missing lines Patch % Lines
...ages/onboarding/knowledge_panel_page_template.dart 0.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #2889   +/-   ##
=======================================
  Coverage     6.94%   6.94%           
=======================================
  Files          229     229           
  Lines        11155   11153    -2     
=======================================
  Hits           775     775           
+ Misses       10380   10378    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@g123k
Copy link
Collaborator

g123k commented Sep 1, 2022

I'm not really fan of it, because it forces to click the tooltip to continue.
Buttons are not accessible anymore (according to your screenshot).

@monsieurtanuki
Copy link
Contributor Author

I'm not really fan of it, because it forces to click the tooltip to continue.

You're right, but it was already the case before. Actually you just had to click anywhere on the screen to dismiss the tooltip, not specifically the tooltip.

Buttons are not accessible anymore (according to your screenshot).

Again, the buttons were NOT accessible. They were visible, but could not be tapped on, unless you clicked before somewhere on the screen to dismiss the tooltip.

Something did not work before, and it still doesn't: I haven't changed that, I've just moved the tooltip downwards.
I could move the tooltip upwards so that the buttons are visible too if you prefer, but still, the first click is for the tooltip.

A solution would be to:

  • make the tooltip clickable (click = dismiss), and remove the previous full screen click capture
  • move the tooltip upwards, just on top of buttons => visible and active buttons

What do you think of that?

@teolemon teolemon added the 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… label Sep 2, 2022
@g123k
Copy link
Collaborator

g123k commented Sep 3, 2022

I'm not really fan of it, because it forces to click the tooltip to continue.

You're right, but it was already the case before. Actually you just had to click anywhere on the screen to dismiss the tooltip, not specifically the tooltip.

I'm note sure of that, clicking on Next would prevent this.
The real problem here is that the buttons are hidden by it.

What about highlighting the nutriscore card instead? Whatever the screen size, there would be enough place for the bottom buttons.

@monsieurtanuki
Copy link
Contributor Author

@g123k

I'm note sure of that, clicking on Next would prevent this.

Then please try again: this happens when we run the test+screenshots, and this happened when I coded this PR.

The real problem here is that the buttons are hidden by it.

That's why I suggested to:

  • make the tooltip clickable (click = dismiss), and remove the previous full screen click capture
  • move the tooltip upwards, just on top of buttons => visible and active buttons

What about highlighting the nutriscore card instead? Whatever the screen size, there would be enough place for the bottom buttons.

You mean the tooltip landing just below the nutriscore card?

Please consider my suggestion too.

…n top

Impacted file:
* `knowledge_panel_page_template.dart`: higher tooltip; removed the "click anywhere on the screen" capture that prevented a direct click on items
@monsieurtanuki
Copy link
Contributor Author

@g123k I've raised a little the tooltip so that the next / previous buttons are visible.
I've also removed the inkwell on top that prevented a direct tap on an item.

Capture d’écran 2022-09-06 à 12 17 36

@monsieurtanuki monsieurtanuki merged commit b163db9 into openfoodfacts:develop Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score…
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Onboarding: misplaced overlay
4 participants