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

[PLAY-1539] Playbook Website Redirects - Go to Parent not Home #4013

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

Tomm1128
Copy link
Contributor

@Tomm1128 Tomm1128 commented Dec 16, 2024

What does this PR do? A clear and concise description with your runway ticket url.

This fixes the redirection of none sense urls to the parent page if there is one otherwise, it sends the user home.

PLAY-1539

Screenshots: Screenshots to visualize your addition/change

How to test? Steps to confirm the desired behavior:

  1. Go to links listed on Story card
  2. Should see appropriate redirects

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY I have added the milano label to show I'm ready for a review.
  • TESTS I have added test coverage to my code.

@Tomm1128 Tomm1128 added improvement This is used when your PR contains library upgrades or doc/site improvements. (USED IN CHANGELOG)) minor Semver Target labels Dec 16, 2024
@Tomm1128 Tomm1128 self-assigned this Dec 16, 2024
@Tomm1128 Tomm1128 requested a review from a team as a code owner December 16, 2024 17:06
@Tomm1128
Copy link
Contributor Author

This is how I setup the redirects:

  • /kit_category/NONSENSE goes to home, or "/"
    • This does not have a parent page so I am keeping the default of redirecting home.
  • /changelog/NONSENSE gives an error
    • This redirects to changelog/web since /changelog already defaults to changelog/web
  • /visual_guidelines/NONSENSE goes to the colors.md but keeps the incorrect path "/visual_guidelines/NONSENSE"
    • Fixed the issue where the url does no reflect the redirect
  • /guides/design_guidelines/NONSENSE gives an error (but will be updated to go to "/guides/design_guidelines" in PLAY-1426
    • Works as expected
  • /samples/NONSENSE goes to "/samples"
    • Works as expected

@Tomm1128
Copy link
Contributor Author

Something I noticed and researched.

Unintended Navigation/Back Button Issues

Issue: By modifying the URL without a corresponding change in the page's content, users might expect that pressing the browser's "Back" button will navigate them to the previous page—but it might just bring them back to the same /visual_guidelines/colors path repeatedly (depending on how many times this code executes).
Fix: Use history.replaceState() instead of pushState() if you don't want to add a new history entry.

@nidaqg nidaqg added the milano 20 MAX - Deploy this PR to a review environment via Milano label Dec 18, 2024
jasperfurniss
jasperfurniss previously approved these changes Dec 18, 2024
@jasperfurniss jasperfurniss added Code Approved Approved by a Playbook Admin and removed Needs Review labels Dec 18, 2024
@Tomm1128
Copy link
Contributor Author

@jasperfurniss - I am thinking of making the change I brought up above. I just tested it in milano and this does add an extra history entry so I am going to try the suggestion above and see if that is a better user experience

@jasperfurniss jasperfurniss added the Code Approved Approved by a Playbook Admin label Dec 18, 2024
@nidaqg nidaqg added Ready for Release merged to master, ready for a versioned released Product Approved pending technical review, OK to merge to master labels Dec 20, 2024
@jasperfurniss jasperfurniss merged commit a5af760 into master Dec 20, 2024
15 checks passed
@jasperfurniss jasperfurniss deleted the play-1539-website-redirects branch December 20, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Approved Approved by a Playbook Admin improvement This is used when your PR contains library upgrades or doc/site improvements. (USED IN CHANGELOG)) milano 20 MAX - Deploy this PR to a review environment via Milano minor Semver Target Product Approved pending technical review, OK to merge to master Ready for Release merged to master, ready for a versioned released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants