Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Consistent website theme and custom 404 #12426

Merged
merged 7 commits into from
Sep 12, 2018

Conversation

aaronmarkham
Copy link
Contributor

@aaronmarkham aaronmarkham commented Aug 31, 2018

Description

The PR applies the website theme to each version. The navigation will be the same, so the option for Clojure needs to be handled properly for old versions. For this I use the .htaccess file to redirect users to an API error page. For good measure, I also added a custom 404 error page.

This PR stacks on #12413 (has the same changes in build_all_version.sh), plus a change to copy the theme, and fixes my concerns there with the theme.

Preview

http://34.201.8.176/
You can test the redirect if you switch to an old version like 1.0.0 and go to API --> Clojure
You can look at the 404:
http://34.201.8.176/error/404.html
If you trigger a 404 by going to an invalid link it goes to the non-existent production URL. I didn't use a relative link because that won't update the URL and this causes formatting issues (described in the convo below) when sidebar.js does all of its wacky document.ready injections. When this PR goes live then it'll all come together.

Comment

I'm sure there's probably some fancy regex that would collapse the clojure rules to one line, but I'll let someone else get fancy.

Copy link
Contributor

@lupesko lupesko left a comment

Choose a reason for hiding this comment

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

Cool!
Left a couple of comments.

docs/error/404.md Outdated Show resolved Hide resolved
docs/error/404.md Outdated Show resolved Hide resolved
@aaronmarkham
Copy link
Contributor Author

For anything in /api like http://34.201.8.176/api/python/index2.html you get
2018-09-07_13-38-49
...which is unexpected to say the least. Every other directory behaves properly.

Please don't merge this until I figure out what's going on and patch it.

@aaronmarkham
Copy link
Contributor Author

aaronmarkham commented Sep 10, 2018

When a request has /api in the URL, sidebar.js triggers and tries to a bunch things to the layout which don't exist on the error page. Using the following would leave the URL intact after a redirect.

ErrorDocument 404 /error/404.html

Rather than try to fix the js, I updated the redirect to use a static URL which forces the URL to change thereby bypassing the theme.

ErrorDocument 404 https://mxnet.incubator.apache.org/error/404.html

So, if you preview this, you won't get to see the error page because it doesn't exist yet. But you can still visit it manually here:
http://34.201.8.176/error/404.html

@aaronmarkham
Copy link
Contributor Author

@lupesko - I made the suggested changes. Can you clear your change request?

Copy link
Contributor

@lupesko lupesko left a comment

Choose a reason for hiding this comment

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

Looks good!

@indhub indhub merged commit cb6c72c into apache:master Sep 12, 2018
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request Sep 19, 2018
* consistent theme plus error handling for missing apis and pages

* add error pages

* update messaging

* force url update to 404 page on redirect

* static redirect to force url update

* version dropdown fix; reverts apache#12482; lesser of evils

* adding note to nudge past flakey test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants