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.

Sign up for GitHub

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

Jump to bottom

feat: Add animation when deleting an item from a list #1411

Merged
merged 4 commits into from
Mar 31, 2022
Merged

feat: Add animation when deleting an item from a list #1411

merged 4 commits into from
Mar 31, 2022

Conversation

IRFAN-GujjAR
Copy link

@IRFAN-GujjAR IRFAN-GujjAR commented Mar 29, 2022 •
edited by VaiTon
Loading

@IRFAN-GujjAR IRFAN-GujjAR requested a review from a team as a code owner March 29, 2022 19:48
@IRFAN-GujjAR IRFAN-GujjAR changed the title Add animation when deleting an item from a list feat: Add animation when deleting an item from a list Mar 29, 2022
@IRFAN-GujjAR
Copy link
Author

Animation for delete list item

@IRFAN-GujjAR IRFAN-GujjAR mentioned this pull request Mar 29, 2022
Closed
@M123-dev M123-dev reopened this Mar 29, 2022
M123-dev
M123-dev reviewed Mar 29, 2022
View reviewed changes
Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Heyy @IRFAN-GujjAR, looks good. Just one small comment

packages/smooth_app/lib/pages/product/common/product_list_page.dart Outdated
@@ -195,7 +195,17 @@ class _ProductListPageState extends State<ProductListPage> {
);
if (dismissible) {
return Dismissible(
background: Container(color: colorScheme.background),
//Delete Animation Added
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this comment here

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//Delete Animation Added

AshAman999
AshAman999 reviewed Mar 30, 2022
View reviewed changes
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.

just some UI suggestions from my side

packages/smooth_app/lib/pages/product/common/product_list_page.dart
@@ -195,7 +195,17 @@ class _ProductListPageState extends State<ProductListPage> {
);
if (dismissible) {
return Dismissible(
background: Container(color: colorScheme.background),
//Delete Animation Added
direction: DismissDirection.endToStart,
Copy link
Member

Choose a reason for hiding this comment

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

is the end to start behavior intended here, i guess it will limit the swipe feature to just one side

packages/smooth_app/lib/pages/product/common/product_list_page.dart
direction: DismissDirection.endToStart,
background: Container(
alignment: Alignment.centerRight,
padding: const EdgeInsets.only(right: 30),
Copy link
Member

Choose a reason for hiding this comment

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

if you switch to a dark theme you will see that the red color container looks bigger than the list tile ,
maybe a margin around the container fix that, you can try the suggested changes

Copy link
Author

Choose a reason for hiding this comment

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

I have added margin of 14 to bottom and top.

M123-dev
M123-dev reviewed Mar 30, 2022
View reviewed changes
packages/smooth_app/lib/pages/product/common/product_list_page.dart Outdated
background: Container(
alignment: Alignment.centerRight,
padding: const EdgeInsets.only(right: 30),
color: Colors.red,
Copy link
Member

Choose a reason for hiding this comment

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

We have our own color scheme in design_constants.dart it would be good if you could switch to that color

Copy link
Author

Choose a reason for hiding this comment

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

I have now changed Colors.red to RED_COLOR from design_constants.dart

@teolemon teolemon added animations 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… labels Mar 30, 2022
@teolemon teolemon linked an issue Mar 30, 2022 that may be closed by this pull request
Add animation when deleting an item from a list #1403
Closed
@IRFAN-GujjAR
Copy link
Author

Now its ready to be merged

M123-dev
M123-dev approved these changes Mar 31, 2022
View reviewed changes
Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @IRFAN-GujjAR

@M123-dev M123-dev merged commit 72bfb99 into openfoodfacts:develop Mar 31, 2022
@github-actions github-actions bot mentioned this pull request Mar 31, 2022
Closed
@IRFAN-GujjAR
Copy link
Author

Welcome @M123-dev

@AshAman999 AshAman999 mentioned this pull request Apr 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AshAman999 AshAman999 AshAman999 left review comments

@VaiTon VaiTon VaiTon left review comments

@M123-dev M123-dev M123-dev approved these changes

Assignees
No one assigned
Labels
animations 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score…
Projects
🤳🥫 The Open Food Facts mobile app (Android & iOS)
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add animation when deleting an item from a list
5 participants
@IRFAN-GujjAR @VaiTon @M123-dev @AshAman999 @teolemon