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: keep item quantity in sync #218

Merged
merged 4 commits into from
Jan 10, 2019
Merged

fix: keep item quantity in sync #218

merged 4 commits into from
Jan 10, 2019

Conversation

jlengstorf
Copy link
Contributor

  • adding more of the same item to the cart updates its quantity input
  • add a basic “loading” animation while quantities update
  • convert the CartListItem component to use Hooks

- adding more of the same item to the cart updates its quantity input
- add a basic “loading” animation while quantities update
- convert the `CartListItem` component to use Hooks
@@ -20,9 +20,9 @@
"gatsby-plugin-sharp": "^2.0.6",
"gatsby-source-shopify": "^2.0.5",
"gatsby-transformer-sharp": "^2.1.3",
"react": "^16.4.2",
"react": "^16.8.0-alpha.0",

Choose a reason for hiding this comment

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

🙊

const target = event.target;
const value = target.value;
const value = Number(target.value);
const updateQuantityDebounced = debounce(500, val => updateQuantity(val));
Copy link

@gaearon gaearon Jan 10, 2019

Choose a reason for hiding this comment

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

I don’t think this does the right thing. You’re starting debouncing from scratch on every input change. So it doesn’t actually debounce as far as I tell. Each change is treated as if it was first.

Instead you probably want to create a debounced setter in a useMemo at the top level. Or in a ref. Then it’s shared between all event handlers in all renders.

Another take on this could be useDebounced(value, ms) which debounces a value rather than the setter. You’d use it like const debouncedQuantity = useDebounced(quantity, ms) and call setQuantity normally. Implementing useDebounced is left as an exercise to the reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I worked on a useDebounced hook, but then I realized that disabling the cart while updating eliminates the need for this to be debounced at all. 😅

handleRemove,
isCartLoading
}) => {
const [quantity, setQuantity] = useState(item.quantity);

Choose a reason for hiding this comment

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

@jlengstorf forgive me if I'm just new to hooks, but I don't think the item quantity will stay sync'd to the corresponding cart list item (fix is missing?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well that’s embarrassing.

Copy link

@amberleyromo amberleyromo left a comment

Choose a reason for hiding this comment

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

👍🏼

@amberleyromo amberleyromo merged commit 91e7ae7 into next Jan 10, 2019
@jlengstorf jlengstorf deleted the fix/quantity-bug branch January 10, 2019 18:14
jlengstorf added a commit that referenced this pull request Jan 11, 2019
* chore: add link to the live store

* fix: update callback domain for Auth0

* feat: store redesign (#201)

TODO:
- [x] ~~refactor `ProductImagesBrowser` (for fixing transitions)~~
- [ ] filtering for `ProductListing` (by swag codes) ()
- [ ] internal Ads functionality
- [x] ~~screens transitions/switching on mobile (some fixes/improvements)~~
- [ ] a couple updates in content
- [x] ~~a list of small fixes and improvements in styles~~

Till the moment I fix the bugs in mobile switching/transitions I suggest testing the store on desktop, instead you will be pointing me strange behaviors I am aware of.

* feat: update used swag code badge style (#212)

* feat: style used badges, modify messaging, update badge theming

* camelcase swag code badge title

* fix: revert static test

* feat: level 2 incentive (#213)

Where a contributor has earned level 1, but not yet level 2, show a progress bar and messaging toward earning level 2.

<img width="420" alt="screen shot 2019-01-06 at 11 52 30 am" src="https://user-images.githubusercontent.com/3461087/50739721-b0828600-11a9-11e9-8b0c-98a6c01e3565.png">

closes #207

* feat: add Google Analytics (#216)

* chore: use official Shopify plugin (#217)

- update to the Gatsby-maintained Shopify plugin (see gatsbyjs/gatsby#10955)
- update the Shopify Buy SDK

* fix: keep item quantity in sync (#218)

- adding more of the same item to the cart updates its quantity input
- add a basic “loading” animation while quantities update
- convert the `CartListItem` component to use Hooks

* feat: use custom domain for login

closes #19

* fix: improve cart button accessibility (#220)

* fix: add button label to shopping cart icon

* fix: reorder components to announce shopping cart earlier
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.

3 participants