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

feat: app redirects replace location #158

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maxmezzomo
Copy link

@maxmezzomo maxmezzomo commented Nov 18, 2024

Closes #154

What I did

Replace app redirects to replace the location.

SKU and SKU Lists apps. Maybe there's more but those are the ones I found.

This prevents infinite redirects if trying to navigate back in history.

How to test

Start the server and click SKUs App, navigation should work with appropriate redirect, try to navigate back in history, you should be back to home. Can repeat for SKU Lists App.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests).
  • Make sure to add/update documentation regarding your changes.
  • You are NOT deprecating/removing a feature.

Copy link

netlify bot commented Nov 18, 2024

Deploy Preview for commercelayer-dashboard-apps ready!

Name Link
🔨 Latest commit 1ba9ccd
🔍 Latest deploy log https://app.netlify.com/sites/commercelayer-dashboard-apps/deploys/673b8ab8252f010008216453
😎 Deploy Preview https://deploy-preview-158--commercelayer-dashboard-apps.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@maxmezzomo
Copy link
Author

if conceptually you are happy with the changes of this PR, I can commit to reset package lock to upstream. Don't know if you have conventions to always update lock but if not would say don't need for this kind of change.

@pfferrari
Copy link
Contributor

Hi Max, good catch! Thanks for your PR, we'll discuss about these changes in the next days and we'll ping you back soon.

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.

SKUs and SKU Lists break navigation
2 participants