-
Notifications
You must be signed in to change notification settings - Fork 3
Redesigned User-Friendly 404 Page #18
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
Conversation
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.
Seems like there's a lot of extraneous code here- why do we need FontAwesome for just one GitHub icon?? Why is Roboto in assets? What is "site pages" in 404?
Also, stop squashing your commits, it's better to have lots of smaller commits rather than one or two massive commits.
|
@dbqeo
|
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.
Sorry, I wasn't being clear, it was late and i was sleepy :PPP
Anyways, I'll make a few things clearer:
- Great 👍
- Since Materialize already uses the Roboto font from their server, you can copy that and it won't require any more loading times or file bloating. https://cdnjs.cloudflare.com/ajax/libs/materialize/0.98.2/fonts/roboto/Roboto-Regular.woff2
- Sorry I didn't catch that, I was on phone and didn't look at it closely enough. That's great 👍
- Squashes like the one you did are fine, it's just that if you are doing lots of things that are unrelated, don't squash them. Several commits to the same file may be squashed. (@EnumC we should write a guide about when to squash in CONTRIBUTING)
Final very important thing:
Angular by default does not actually use a 404 page. I'll make a commit to the branch to make it do so.
TL;DR I'll make sure the 404 redirects properly; everything seems fine except for the duplicate Roboto.
|
@EnumC Please verify everything is working and merge when ready. Thanks! |
Static Standalone 404 Page - Would Not Affect Load Time.