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

Is it possible to show one route over another? #408

Open
abdonrd opened this issue Oct 23, 2019 · 7 comments
Open

Is it possible to show one route over another? #408

abdonrd opened this issue Oct 23, 2019 · 7 comments
Labels
enhancement New feature or request

Comments

@abdonrd
Copy link
Contributor

abdonrd commented Oct 23, 2019

My use case is:

I have a page-user element for the route /user/:id.
In this element, I request the user info in the onBeforeEnter callback using the location.params.id.
When the request return the user, everything is fine. But if the user doesn't exits, I want to show the page-not-found element (a 404 page) without having to redirect or change the route.

Is this possible to do?

@vlukashov
Copy link
Contributor

This is something you could get working with a custom route action, but I do see an easy way to achieve that when using the route.component property:

{ path: "/users/:id",
  action: async(context, commands) => {
    // simulates an async fetch operation
    await new Promise(resolve => setTimeout(resolve, 1000));

    if (context.params.id === 'sam') {
      return commands.component('profile-view');
    } else {
      return commands.component('not-found-view');
    }
  }
},

Here is a demo: https://glitch.com/edit/#!/408-dynamic-route-componenet?path=script.js:11:9

The downside of this approach is that the router displays a blank page while the async action is in progress.

An alternative solution would be to change the profile-view component so that it handles the situation itself and either renders an loading state, or user profile state, or no such user state, but that would not involve the router. It may be more complex to implement, but would result in a better UX because the user would always immediately see some content.

@abdonrd
Copy link
Contributor Author

abdonrd commented Oct 24, 2019

Yes, leaving the screen blank while the request is not resolved is not a valid option for us. 😕

In fact, right now we are making the data requests from the onBeforeEnter router callback. Example:

const GET_USER = gql`
  query GetUser($id: ID!) {
    user(id: $id) {
      username
      fullName
    }
  }
`;

@customElement('page-user')
export class PageUser extends connectApollo(client)(PageElement) {
  // ...

  protected onBeforeEnter(location: Router.Location) {
    // When `this.requestQuery` resolves, it populates `data` and `loading` properties
    this.requestQuery({
      query: GET_USER,
      variables: {
        id: location.params.id
      }
    });
  }

  protected shouldUpdate() {
    if (this.data && this.data.user && !this.loading) {
      // TODO: Show page-not-found element
    }

    return true;
  }
}

@vlukashov
Copy link
Contributor

The approach you describe looks very reasonable to me. The page-not-found is one of the three possible states of the page-user view.

The router has no knowledge of the internal logic of the component, it renders the page-user components for the /users/:id URL and lets the component decide which state to render when.

Do you have a specific feature request in mind? I am not sure how a router can help you here?

@abdonrd
Copy link
Contributor Author

abdonrd commented Oct 24, 2019

With this scenario, I have two ideas in mind:

  1. The page-user render a custom no such user state.
    We can do it right now, it's easy.

  2. Some way to force the router render the page-not-found element without a redirect?
    I think this could make sense because it's interesting to be able to share a page-not-found among many pages. Imagine also have another pages like page-article, page-category, etc. In fact, is the same element that we declare in the router for the fallback route ('(.*)')

Sometimes it will be interesting to have a custom not-found page, but other times it will be interesting to share the same not-found page.

@vlukashov
Copy link
Contributor

The feature as I understand it is about managing the route component's state: loading / loaded OK / loaded not OK. I can see how this feature may by useful in many apps, so it could be considered for the Router roadmap in the future.

As a workaround now I would suggest explicitly re-using the page-not-found component in every view that needs it. E.g.:

render() {
  if (this.loading) {
    return html`Loading...`;
  } else if (this.user) {
    return html`
    <p>ProfileView</p>
    ...
  `;
  } else {
    return html`<not-found-view></not-found-view>`;
  }
}

See a demo here: https://stackblitz.com/edit/408-three-state-component?file=views%2Fprofile-view.js

@vlukashov vlukashov added the enhancement New feature or request label Oct 25, 2019
@abdonrd
Copy link
Contributor Author

abdonrd commented Oct 28, 2019

Thank you @vlukashov! For now I will do that.

I imagine something like Router.render('page-not-found') in the future.
Where page-not-found is the name of the route.

@web-padawan
Copy link
Member

One thing to consider is that navigation should probably still point to the correct page, see the example:

notfound

As you can see, unlike the real 404 page, user still has the visual context (the "dashboard" view is active).

This can be achieved differently, depending on the way the app is implemented:

  1. If the navigation is placed outside the router outlet (e.g. in the app shell), then you can use the above code example.
  2. If every view is responsible of rendering navigation, then "page not found" should be rendered by the view itself.

In the above example (the app I used to work on), the second approach is used. IMO it is more flexible, because it allows to use different layouts, e.g. with or without drawer, depending on the view - for example, in case of unauthorised user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants