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 pagination error and update tracking info #645

Merged
merged 4 commits into from
Sep 24, 2019

Conversation

melissaluu
Copy link
Contributor

@melissaluu melissaluu commented Sep 19, 2019

Currently, when a product set is initialized (i.e. an array of product ids), there is a pagination console error (since product sets do not have connections, it cannot be paginated). This PR fixes this issue by only showing pagination for collections and not product sets.

Also, the tracking info objects passed to the tracker are outdated and some properties are incorrect - this PR will update the tracking info and will also standardize the id property to always return the storefrontId.

🎩 instructions:

  • load a product set buy button and ensure that there are no pagination console error and pagination does not appear in the UI
  • load a collection buy button and ensure that pagination is still available and works as expected

Copy link
Contributor

@spencercanner spencercanner left a comment

Choose a reason for hiding this comment

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

One small comment, but looks good otherwise
🎩'ed in Chrome and Safari

src/components/product-set.js Outdated Show resolved Hide resolved
@melissaluu melissaluu changed the title Fix pagination error and update tracking info for product sets Fix pagination error and update tracking info Sep 23, 2019
Copy link
Contributor

@spencercanner spencercanner left a comment

Choose a reason for hiding this comment

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

🎩'ed the new changes in chrome
Looks good! 🚀

@melissaluu melissaluu merged commit 15e0fd8 into master Sep 24, 2019
@melissaluu melissaluu temporarily deployed to production September 24, 2019 20:19 Inactive
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.

2 participants