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: #2846 horizontal layout buttons #2899

Merged
merged 7 commits into from
Sep 5, 2022

Conversation

abughalib
Copy link
Contributor

What

image

Part of

fixes #2846

@abughalib abughalib requested a review from a team as a code owner September 1, 2022 18:56
Copy link
Member

@AshAman999 AshAman999 left a comment

Choose a reason for hiding this comment

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

LGTM :)
Ty @abughalib

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.

Hi @abughalib!
The code looks code, but the screenshot does not, as there's no space between the "save" button and the right side of the screen.
Could you please add screenshots for the 3 cases, and add horizontal spaces if needed?

@abughalib
Copy link
Contributor Author

@monsieurtanuki
Screenshot_1662306293

@monsieurtanuki
Copy link
Contributor

Thank you @abughalib!
Can you share with us the 3 screenshots? The one you attached recently is better but not good: the space between the save button and the right edge is smaller than the space between the card and the right edge. The spaces should be identical.

@M123-dev
Copy link
Member

M123-dev commented Sep 4, 2022

I'd prefer to not block this pr, having a bit off spacing is better then the whole buttons not aligned

@monsieurtanuki
Copy link
Contributor

I'd prefer to not block this pr, having a bit off spacing is better then the whole buttons not aligned

@M123-dev I don't block anything: I just give my opinion as required.
And the PR is already approved by someone else, so feel free to approve too or to merge.

@abughalib
Copy link
Contributor Author

Screenshots.
Screenshot_1662384323
Screenshot_1662384334
Screenshot_1662384383

@codecov-commenter
Copy link

Codecov Report

Merging #2899 (b3b164a) into develop (d9c35d7) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #2899      +/-   ##
==========================================
- Coverage     6.71%   6.70%   -0.01%     
==========================================
  Files          228     228              
  Lines        11117   11118       +1     
==========================================
  Hits           746     746              
- Misses       10371   10372       +1     
Impacted Files Coverage Δ
...h_app/lib/pages/product/edit_ingredients_page.dart 0.00% <ø> (ø)
...h_app/lib/pages/product/nutrition_page_loaded.dart 0.00% <ø> (ø)
...mooth_app/lib/pages/product/simple_input_page.dart 0.00% <0.00%> (ø)

📣 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 @abughalib!

@AshAman999 AshAman999 merged commit 91aa457 into openfoodfacts:develop Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout issue in edit mode for Save and Cancel buttons
5 participants