-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
Implement 404 page #70
Comments
If anyone wants to pick this up they might want to check out how we do this in the guides-app https://github.com/ember-learn/guides-app/blob/master/app/templates/error.hbs 👍 |
We should consider consuming the template from ember style guide app. ie everyone gets 404 page for free when using the add-on. |
@sivakumar-kailasam does that exist? or is your suggestion "add the 404 to the ember-styleguide and then add it to deprecation-app"? |
I'm unsure if it exists. My suggestion is to add anything that's going to be required across ember apps to go directly into style guide going forward and have consuming apps get it because they all point to styleguide's master |
very rough wip: https://github.com/ember-learn/deprecation-app/compare/master...amyrlam:serena-amy/404?expand=1 followed this https://discuss.emberjs.com/t/how-to-make-a-404-router/11349 unsure what the route handler should do? me and @serenaf can keep working on this |
@amyrlam thanks for taking a stab a this and sorry for not getting back to you before now 😖 If you take a look at what is happening in the guides app https://github.com/ember-learn/guides-app/blob/master/app/templates/error.hbs we're not actually adding any route to the router to get the 404 implementation to work (i.e. I would start on removing this line) What we're making use of is the default/standard error substate: https://guides.emberjs.com/v3.0.0/routing/loading-and-error-substates/#toc_code-error-code-substates. We had a long discussion on the merits of both approaches in the Guides App and ultimately opted for the error substate for overall simplicity and more consistency with what we are teaching in the Guides. Let me know if you would like any help exploring this option 👍 |
sorry for the delay - that makes sense, can pick this back up with serena, just took a look at this pr https://github.com/ember-learn/guides-app/pull/24/files |
@rwwagner90 Thanks - please go ahead, this is pretty stale. I haven't been in this repo in some time |
The text was updated successfully, but these errors were encountered: