-
-
Notifications
You must be signed in to change notification settings - Fork 112
feat: convert redirect page to inertia #4076
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: convert redirect page to inertia #4076
Conversation
# Conflicts: # lang/en_US.json
wescopeland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do an exhaustive review either later this evening or tomorrow once checks are passing, but wanted to shoot along one quick comment.
# Conflicts: # lang/en_US.json
|
Looks like we've got a coverage gap in Vitest:
PHPUnit is upset about the controller returning Inertia stuff rather than a Blade view. I wonder if |
|
I'll take a look at that, |
# Conflicts: # lang/en_US.json
|
Still looks like we're having some trouble with the PHPUnit test suite: https://github.com/RetroAchievements/RAWeb/actions/runs/19050202919/job/54408281395?pr=4076 Want me to try to poke around? |
|
Yeah I'm not sure where the issue is coming from. These tests run fine locally for me |
|
They all pass for me too. I wonder if this might fix it, would you mind trying this solution? // config/inertia.php
// line 50
resource_path('js/pages'),"Pages" -> "pages". |
I do see the errors on my VM and this prescribed change does make them go away. |
|
I think it might be a case sensitivity issue since I use Mac and it does not have case sensitive folders like Linux (and presumably Windows). |
# Conflicts: # resources/js/ziggy.js
resources/js/features/redirect/components/+root/RedirectRoot.tsx
Outdated
Show resolved
Hide resolved
resources/js/features/redirect/components/+root/RedirectRoot.tsx
Outdated
Show resolved
Hide resolved
resources/js/features/redirect/components/+root/RedirectRoot.tsx
Outdated
Show resolved
Hide resolved
resources/js/features/redirect/components/+root/RedirectRoot.test.tsx
Outdated
Show resolved
Hide resolved
resources/js/features/redirect/components/+root/RedirectRoot.test.tsx
Outdated
Show resolved
Hide resolved
# Conflicts: # lang/en_US.json
# Conflicts: # lang/en_US.json # resources/js/ziggy.js
Very basic for now. Doesn't change anything, but open to ideas for improvements.
Only notable change is instead of a
<a href>it useswindow.location.replace, which will make pressing "Back" on the browser go to the original page instead of this redirect page.Also, of course, a test and i18n keys.
Redirect's route was moved into an Inertia middleware so it could work, it could be moved elsewhere if needed.
Preview, if relevant.