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: Made the notification bar of the scanner transluent #4611

Merged
merged 1 commit into from
Sep 2, 2023

Conversation

sm-sayedi
Copy link
Contributor

@sm-sayedi sm-sayedi commented Aug 30, 2023

What

  • The notification bar (status bar) in the scan page which looks opaque, is made transluent in this fix.

Screenshots

Before

Dark Light

After

Dark mode Light Mode

Fixes bug(s)

Impacted Files

  • scan_page.dart

@sm-sayedi sm-sayedi requested a review from a team as a code owner August 30, 2023 11:19
@github-actions github-actions bot added 🥫 Product page 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… Android dependencies 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… labels Aug 30, 2023
@monsieurtanuki
Copy link
Contributor

Hi @sm-sayedi!
We haven't migrated to flutter 3.13 yet - there's a pending PR about it.
Therefore most of your changes are not compatible with "our" 3.10.

@sm-sayedi
Copy link
Contributor Author

@monsieurtanuki So it means this PR is postponed until the project is migrated to Futter 3.13, am I right?!

@monsieurtanuki
Copy link
Contributor

@monsieurtanuki So it means this PR is postponed until the project is migrated to Futter 3.13, am I right?!

@sm-sayedi That's correct.
Other solution: remove from this PR all the files related to flutter 3.13 (or create a fresh PR if you prefer), as your contribution to solve the issue was in a file not concerned by the migration, if I'm not wrong.

@github-actions github-actions bot removed 🥫 Product page 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… dependencies labels Aug 31, 2023
@sm-sayedi
Copy link
Contributor Author

@monsieurtanuki I have updated my PR. Just the file related to the bug is included.

@monsieurtanuki
Copy link
Contributor

Thank you @sm-sayedi for your code change!

@teolemon Would you please check that the screenshots match you expectations? (especially the After / Normal one).
Tbh I don't think I understand the goal of the issue and therefore cannot assess @sm-sayedi's work.
If we remove the top from the SafeArea (as currently coded), the scan controls go on top of the standard top data.

@monsieurtanuki
Copy link
Contributor

@sm-sayedi I think I've finally understood what is needed, and it looks like #4606 (comment).
I believe you have to remove the SafeArea, as it's already embedded in the SmoothScaffold(and using another SafeArea may have side effects).
The goal is still to display the standard top bar (but on a transparent background dealt with by SmoothScaffold), and then the camera. At least that's what I've understood...

@sm-sayedi
Copy link
Contributor Author

@monsieurtanuki So it means that the visual result should be the same but by using the SmoothScaffold and removing the SafeArea?

@monsieurtanuki
Copy link
Contributor

@monsieurtanuki So it means that the visual result should be the same but by using the SmoothScaffold and removing the SafeArea?

@sm-sayedi It means that:

  • the result would be more like what is expected (the standard status bar, and the camera just below)
  • you don't need an additional SmoothScaffold, it's already there
  • indeed you do need to remove the SafeArea from the page

@monsieurtanuki
Copy link
Contributor

@sm-sayedi From what I've understood in #4615, the problem is slightly different.

On light mode, the status bar is already translucent, because the standard background is white (more or less).
On dark mode, the status bar is "not translucent", because the standard background is black (more or less).
Actually, the issue is only on dark mode.

The solution is to:

  • start with a Container with a white background => translucent status bar
  • then a SafeArea => safe area respected
  • then using the standard background color for the bottom of the screen (so that we don't see a light background in dark mode)

Do you feel like coding it? I can do it if needed.

For the record, that's an example of a translucent top bar in dark mode - as you see, we need to switch back to a dark color for the bottom of the body:
Screenshot_2023-09-01-14-56-24

@sm-sayedi
Copy link
Contributor Author

@monsieurtanuki Thanks for describing the situation! For now I want to do something like this:

Container(
        color: Colors.white, // For making the status bar translucent
        child: SafeArea(
          child: Container(
            color: Theme.of(context).colorScheme.background, // For making the body dark again
            child: Column(

Now the problem is that I don't exacly know what is the proper backgroundColor for the body. For now I have used Theme.of(context).colorScheme.background. The result is as following:

Which is slightly different from the color that the dark mode applies by default:

As you can see, the background color in the second screenshot is the same as the background color of the navigation bar.
Is this okay or should I use another color? I would appreciate to know how to get the exact color that the dark mode applies to other widgets!

@monsieurtanuki
Copy link
Contributor

@sm-sayedi Indeed Theme.of(context).colorScheme.background is the appropriate color.
Please push the latest version of your code and attach one light and one dark screenshot.

@sm-sayedi
Copy link
Contributor Author

@monsieurtanuki It's done! The screenshots are also updated!
Can you also please assign me to this issue?!

@codecov-commenter
Copy link

Codecov Report

Merging #4611 (4619850) into develop (1bae5a7) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4611      +/-   ##
===========================================
- Coverage    10.14%   10.14%   -0.01%     
===========================================
  Files          299      299              
  Lines        15686    15689       +3     
===========================================
  Hits          1591     1591              
- Misses       14095    14098       +3     
Files Changed Coverage Δ
packages/smooth_app/lib/pages/scan/scan_page.dart 2.22% <0.00%> (-0.08%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sm-sayedi for your first contribution! Congratulations!

@sm-sayedi
Copy link
Contributor Author

Thank you @monsieurtanuki and @teolemon! It was my first open-source contribution ever, and it was awesome to start with smooth-app!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion…
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the notification bar of the scanner transluent
3 participants