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

@W-16660178@ Small bug fixes #2003

Merged
merged 6 commits into from
Sep 11, 2024
Merged

@W-16660178@ Small bug fixes #2003

merged 6 commits into from
Sep 11, 2024

Conversation

vcua-mobify
Copy link
Contributor

@vcua-mobify vcua-mobify commented Sep 9, 2024

This PR fixes the bugs reported in #1946 and #1941

Testing:

For #1946 (Default product-scroller skips tiles (desktop)),

Start the app on the home page then scroll down to the recommended product scroller. Scroll the products and make sure no tiles are skipped

Before:

Screen.Recording.2024-09-09.at.3.47.18.PM.mov

After:

Screen.Recording.2024-09-09.at.3.54.26.PM.mov

For #1941 (Error handling issues)

PDP / PLP error handling:
Make the app throw a non 404 error and verify that the error page is rendered.
(I just commented out the HTTP 404 case so that the default HTTPError constructor is called when I render a 404)

Navigating back to the home page from error page
Once on the error page, click the Salesforce cloud icon to navigate back to home. Ensure that the home page renders.

Before:

Screen.Recording.2024-09-09.at.4.10.15.PM.mov

After:

Screen.Recording.2024-09-09.at.4.47.10.PM.mov

@vcua-mobify vcua-mobify requested a review from a team as a code owner September 9, 2024 23:41
Comment on lines 55 to 53
onClick={() => history.push('/')}
onClick={() => (window.location.href = '/')}
Copy link
Collaborator

Choose a reason for hiding this comment

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

So that I understand clearly -- the idea here is that the application is in an error state from a JavaScript perspective, so in some sense "React has crashed" so the window.history listener in React doesn't pick up the change so we just hard navigate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's correct. If we use history, we trigger a soft navigation and are still in an error state so the home page is not rendered. We need to trigger a hard navigate to get out of the error state.

bfeister
bfeister previously approved these changes Sep 10, 2024
Copy link
Collaborator

@bfeister bfeister left a comment

Choose a reason for hiding this comment

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

Beautiful 💅

alexvuong
alexvuong previously approved these changes Sep 10, 2024
@alexvuong
Copy link
Collaborator

Left a small comment, the rest looks good. Thanks for working on these issues. 👍

@vcua-mobify vcua-mobify dismissed stale reviews from alexvuong and bfeister via 9bd4120 September 10, 2024 22:57
@vcua-mobify vcua-mobify merged commit 18d3085 into develop Sep 11, 2024
29 checks passed
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