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

Adds link to GitHub Repo on 404 page, makes styling respect current theme #199

Merged
merged 5 commits into from
Feb 29, 2020

Conversation

mattxwang
Copy link
Collaborator

This is a pretty small and quick PR. Adds a link to our GitHub repo on our 404 page (which might happen eventually with the view-only programs), and makes the page styling respect the current theme (with the themify mixin). Also tidies up a bit of the underlying React code.

(note: this should be squash merged, commit history is not too useful).

@mattxwang mattxwang added the feature New feature or request label Feb 12, 2020
Copy link
Contributor

@krashanoff krashanoff left a comment

Choose a reason for hiding this comment

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

Hey Matt, looks good, but when you mention the styling should respect the current theme, I only ever get the light themed error page. This was tested on the netlify deploy:

image

Then going to a nonexistent page:

image

Alternatively, going from light mode:

image

...to the same nonexistent page:

image

@mattxwang
Copy link
Collaborator Author

Hey Matt, looks good, but when you mention the styling should respect the current theme, I only ever get the light themed error page.

Good catch! The theme itself wasn't actually working, so that makes lots of sense. Just added on a commit that should properly respect the theme.

(in this case, I'm fine with pulling from cookies since getting nothing/invalid values has sensible defaults)

Copy link
Contributor

@krashanoff krashanoff left a comment

Choose a reason for hiding this comment

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

Nice work Matt, LGTM!

@mattxwang mattxwang merged commit a2b82cb into master Feb 29, 2020
@mattxwang mattxwang deleted the more-descriptive-error-page branch February 29, 2020 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants