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

feat(notfound): add NotFound component #1672

Merged
merged 3 commits into from
May 7, 2018

Conversation

dizco
Copy link
Contributor

@dizco dizco commented May 2, 2018

Please read and mark the following check list before creating a pull request (check one with "x"):

Short description of what this resolves:

Redirects users who hit an unknown page to a custom component
Fixes #1410

image

A couple of things I was not too sure about :

  • I've designed it so that only errors within /pages/* are captured by this. I think that for most single page applications, you probably don't want to catch 404's in a global manner (it's ambiguous if the user is not authenticated). I expect most people to have protected their pages directory with a guard of some sort.
  • Should this component be within nebular?
  • Should we have a direct link in the demo menu to show this page?

Redirects users who hit an unknown page to a custom component
Fixes akveo#1410
@nnixaa
Copy link
Collaborator

nnixaa commented May 3, 2018

Hey @dizco, great addition!

  • yes, I agree, let's keep it for /pages/* only
  • probably not, as this is more of a page rather than component
  • yes, let's add a Pages section and this one as a sub-page. Just for demo purposes.

Also, I would suggest a bit of a refactoring - let's move the comonent from @theme/ to the pages/other (didn't think of a better folder name rather than other...) where it belongs better. As this component more of a 'page' component, refferenced in the routes.

@@ -35,6 +36,9 @@ const routes: Routes = [{
path: '',
redirectTo: 'dashboard',
pathMatch: 'full',
}, {
path: '**',
component: PageNotFoundComponent,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, let's name it NotFoundComponent as the other pages don't have a Page prefix.

@dizco
Copy link
Contributor Author

dizco commented May 3, 2018

@nnixaa what would you think of Miscellaneous instead of Other? Also thought of Extra.

@nnixaa
Copy link
Collaborator

nnixaa commented May 3, 2018

Miscellaneous sounds good!

Added a direct link in the menu, and move the component to Miscellaneous
folder
@dizco
Copy link
Contributor Author

dizco commented May 3, 2018

Choice of icons isn't my particular strength, so please feel free to suggest something better if there is haha!

image

@dizco dizco changed the title feat(notfound): add PageNotFound component feat(notfound): add NotFound component May 3, 2018
import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';

import {MiscellaneousComponent} from './miscellaneous.component';
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dizco let's add the spaces

@nnixaa
Copy link
Collaborator

nnixaa commented May 7, 2018

@dizco cool! Merging now.

@nnixaa nnixaa merged commit fa3cdf7 into akveo:master May 7, 2018
@dizco dizco deleted the dizco-page-not-found branch May 7, 2018 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add 404 error page
2 participants