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: 4206 - search page misalignment in search-history_view.dart (#4210) #4266

Merged
merged 7 commits into from
Jul 7, 2023

Conversation

tanishq5414
Copy link
Contributor

Impacted file:

search-history_view.dart: added padding to the list tile for proper alignment Resolved the misalignment issue on the search page by adding appropriate padding to the list tile in the search-history_view.dart file. This adjustment ensures that the elements within the list are properly aligned, improving the visual consistency of the search page.

What

  • Added padding to the list tile in the search-history_view.dart file to achieve proper alignment.
  • This adjustment ensures that the elements within the list are properly aligned, improving the visual consistency of the search page.

Screenshot

Screenshot_1688317600

Fixes bug(s)

Part of

…nfoodfacts#4210)

Impacted file:

search-history_view.dart: added padding to the list tile for proper alignment
Resolved the misalignment issue on the search page by adding appropriate padding to the list tile in the search-history_view.dart file. This adjustment ensures that the elements within the list are properly aligned, improving the visual consistency of the search page.
@tanishq5414 tanishq5414 requested a review from a team as a code owner July 2, 2023 17:08
@github-actions github-actions bot added the 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… label Jul 2, 2023
@g123k
Copy link
Collaborator

g123k commented Jul 2, 2023

Hello, thanks for your PR.
But it seems that it's not perfectly aligned in your screenshot:
250375133-b2060942-9347-44d0-9e5a-9022d33efdab

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

Furthermore, before pushing, please ensure to format your code.
Thanks

@tanishq5414
Copy link
Contributor Author

Sorry mb, I will make the necessary changes
Does this look good?
image

@g123k
Copy link
Collaborator

g123k commented Jul 2, 2023

Far better 👍

@tanishq5414 tanishq5414 closed this Jul 3, 2023
@tanishq5414 tanishq5414 force-pushed the develop branch 2 times, most recently from 5fddf47 to c818bb1 Compare July 3, 2023 01:32
@github-actions github-actions bot removed the 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… label Jul 3, 2023
…nfoodfacts#4210)

Impacted file:

search-history_view.dart: added padding to the list tile for proper alignment
Resolved the misalignment issue on the search page by adding appropriate padding to the list tile in the search-history_view.dart file. This adjustment ensures that the elements within the list are properly aligned, improving the visual consistency of the search page.
@tanishq5414 tanishq5414 reopened this Jul 3, 2023
@github-actions github-actions bot added the 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… label Jul 3, 2023
@tanishq5414
Copy link
Contributor Author

Hey, I have made the changes. Can you verify?

@g123k
Copy link
Collaborator

g123k commented Jul 3, 2023

Almost perfect, the Ripple effect should take the full width:
Search.webm

Refactored the ripple effect in the search-history_view.dart file to enhance the visual feedback when interacting with list tiles
@tanishq5414
Copy link
Contributor Author

Added the ripple effect to the complete List tile. Please verify and let me know if there are any other changes.

child: Padding(
padding: const EdgeInsets.only(left: 18, right: 21),
child: ListTile(
contentPadding: EdgeInsets.only(left: 18, right: 21),
Copy link
Collaborator

Choose a reason for hiding this comment

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

By curiosity, why do set twice the padding?
Also, by consistency, could you use floating values: 18 -> 18.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the code to follow consistency.
Also, my bad I was testing something and it slipped right through , will be careful before pushing next time

Refactored the code to use floating values in the search-history_view.dart file
Refactored the code to use floating values in the search-history_view.dart file
@tanishq5414
Copy link
Contributor Author

The fix is done ig. It is ready to merge

@g123k
Copy link
Collaborator

g123k commented Jul 4, 2023

It seems that there is still an issue: https://share.waldo.com/294ab6ee641dd9f72e008124c281df26024024?tab=timeline&startTime=39.688

As you can see, the Ripple around the pen (Edit mode) is really small. There should be some padding

@tanishq5414
Copy link
Contributor Author

Did the fix!

@tanishq5414 tanishq5414 requested a review from g123k July 7, 2023 09:29
Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

That's better like so, I will probably implement the accessibility feature later.
Thanks for your PR!

@g123k g123k merged commit 505c0d1 into openfoodfacts:develop Jul 7, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤳🥫 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.

Search page: everything is misaligned
2 participants