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

[EP-2447] Preserve cart order after edit #1081

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Conversation

canac
Copy link
Contributor

@canac canac commented Dec 13, 2023

Preserve the order of items in the cart after changing one. I'm not sure why the previous code was removing the edited item from the UI, so I removed it. The changed item with the old amount will remain in the list briefly, but the loading indicators will be active until the list refreshes, so I believe that is OK.

Tested in staging ✅

https://jira.cru.org/browse/EP-2447

@canac canac added the On Staging Will be merged to the staging branch by Github Actions label Dec 13, 2023
@canac canac requested review from wrandall22 and dr-bizz December 13, 2023 22:27
src/app/cart/cart.component.js Outdated Show resolved Hide resolved
src/app/cart/cart.component.js Show resolved Hide resolved
src/app/cart/cart.component.js Show resolved Hide resolved
src/app/cart/cart.component.js Show resolved Hide resolved
src/app/cart/cart.component.js Outdated Show resolved Hide resolved
@canac canac force-pushed the 2447-preserve-cart-order branch from 09b325c to 12a157c Compare December 18, 2023 14:22
@canac canac requested review from wrandall22 and dr-bizz December 18, 2023 15:18
@wrandall22
Copy link
Contributor

Code looks good, but I'd like to postpone until after peak giving.

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

Your code looks great! 💯 🔥
One place I can see we can improve is by adding skeletons to the Images in the cart, when the cart items refresh. Currently, after an item is edited or removed the images get reloaded, causing the item's titles to have layout shift to the left.
If we added a skeleton it would show the user the items are refreshing and it would stop the layout shift.
This might be a separate PR, but it could be grouped with this one.

@canac
Copy link
Contributor Author

canac commented Dec 28, 2023

@dr-bizz I explicitly set the image dimensions to avoid layout shift. For the record, I only see the layout shift with DevTools open and "Disable cache" checked. Thanks to caching of the images, I don't think that users are experiencing that layout shift.

@dr-bizz
Copy link
Contributor

dr-bizz commented Jan 10, 2024

@dr-bizz I explicitly set the image dimensions to avoid layout shift. For the record, I only see the layout shift with DevTools open and "Disable cache" checked. Thanks to caching of the images, I don't think that users are experiencing that layout shift.

Closing the dev tools seems to ensure it no longer has layout shifts. Thanks

@canac canac force-pushed the 2447-preserve-cart-order branch from 37df5e2 to bc07bb4 Compare January 17, 2024 20:56
@canac canac dismissed wrandall22’s stale review January 17, 2024 21:00

Code freeze is over

@canac canac merged commit acf4225 into master Jan 17, 2024
6 checks passed
@canac canac deleted the 2447-preserve-cart-order branch January 17, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants