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

Handle Github pages 404 issue #2179

Merged
merged 10 commits into from
Nov 11, 2024
Merged

Handle Github pages 404 issue #2179

merged 10 commits into from
Nov 11, 2024

Conversation

jitu5
Copy link
Contributor

@jitu5 jitu5 commented Nov 7, 2024

Description

Resolves #2048

This pull request includes changes to fix a 404 error on the experiment tracking page, and to ensure the 404 page is correctly redirect to root and manage by React router internally by keeping the content of the index.html and 404.html same.

Development notes

Redirect handling:

  • package.json: Modified the build script to copy index.html to 404.html to ensure the 404 page is correctly served.

QA notes

I have deployed the changes to Github pages https://jitu5.github.io to test.

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Jitendra Gundaniya <[email protected]>
@jitu5 jitu5 self-assigned this Nov 7, 2024
@jitu5 jitu5 added the Javascript Pull requests that update Javascript code label Nov 7, 2024
@jitu5 jitu5 marked this pull request as ready for review November 7, 2024 16:18
Signed-off-by: Jitendra Gundaniya <[email protected]>
@astrojuanlu
Copy link
Member

Watching this in slow motion was... interesting 😄

kedro-viz-404

On the other hand, actual faulty URLs are handled in a strange way now:

image

Compare that with how it's handled locally:

image

I think this is smart but I fear it might have some unintended consequences...

@jitu5
Copy link
Contributor Author

jitu5 commented Nov 8, 2024

@astrojuanlu In slow motion you show that text "// in 404.html // in 404.html" was just for testing, I forgot to deploy the latest one , but now its updated. now it shows "Redirecting..."
Screenshot 2024-11-08 at 11 15 33 a m

@jitu5
Copy link
Contributor Author

jitu5 commented Nov 8, 2024

@astrojuanlu

On the other hand, actual faulty URLs are handled in a strange way now:

I fixed it now, if any sub path other than experiment-tracking is hit directly like this https://jitu5.github.io/experiment-trackingadsd, now it will be redirect to home/landing page.

@jitu5
Copy link
Contributor Author

jitu5 commented Nov 8, 2024

@astrojuanlu

Compare that with how it's handled locally:

This approach is specifically added for Github pages, on hitting faulty URLs it redirect you to our custom 404.html but locally thing works same as before.

Signed-off-by: Jitendra Gundaniya <[email protected]>
Signed-off-by: Jitendra Gundaniya <[email protected]>
# Conflicts:
#	RELEASE.md
Signed-off-by: Jitendra Gundaniya <[email protected]>
# Conflicts:
#	RELEASE.md
Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

Look's great now! thanks @jitu5 .. this was a tough one to fix, and u did it in a minimal way!

Copy link
Member

@tynandebold tynandebold left a comment

Choose a reason for hiding this comment

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

Did you try using the exact same contents of the index.html file for the 404.html?

I just had to solve this issue for AlloyViz, which is also deployed to GH pages.

This is how I did it:

"build": "tsc && vite build && cp ./dist/index.html ./dist/404.html",

Might not be the same. Wanted to share my findings in case they were helpful!

@jitu5
Copy link
Contributor Author

jitu5 commented Nov 11, 2024

Did you try using the exact same contents of the index.html file for the 404.html?

I just had to solve this issue for AlloyViz, which is also deployed to GH pages.

This is how I did it:

"build": "tsc && vite build && cp ./dist/index.html ./dist/404.html",

Might not be the same. Wanted to share my findings in case they were helpful!

@tynandebold Thanks for sharing your finding.
But using same content for 404.html from index.html is redirecting to landing page or root but, how you retain the original url and its params entered by user and then redirect to the same using client-side React-router ?

This is even clear solution, I got once the control goes to client-router(React-router) it handle as normal button click or history.push(url).
I will update the PR. Thanks @tynandebold again.

Signed-off-by: Jitendra Gundaniya <[email protected]>
Copy link
Contributor

@Huongg Huongg left a comment

Choose a reason for hiding this comment

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

amazing, all looking good for me 👍 thank you @jitu5

@jitu5 jitu5 merged commit 852e1e1 into main Nov 11, 2024
11 checks passed
@jitu5 jitu5 deleted the fix/ghp-404 branch November 11, 2024 14:11
@jitu5 jitu5 mentioned this pull request Nov 20, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URL Access to Experiment Tracking Causes 404 Error on viz demo
5 participants