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

Blazor: More flexible Router / layout display #10493

Closed
8 tasks done
SteveSandersonMS opened this issue May 23, 2019 · 6 comments
Closed
8 tasks done

Blazor: More flexible Router / layout display #10493

SteveSandersonMS opened this issue May 23, 2019 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented May 23, 2019

Currently, we have a hardcoded set of assumptions about what you want to do with a router, i.e.,

  • You want to display the page inside its layouts
  • You want to authorize access to the page, but still show its layouts regardless

The logic for this is all internal to Router and LayoutDisplay (being renamed to PageDisplay). Instead of this, we could reduce Router to be a templated component that just finds a page and parameters for you. Then you could choose how to render that, and where authorization wrappers go, etc., yourself.

Example:

<Router AppAssembly="@typeof(App).Assembly" Context="routeContext">
    <NotFoundContent>
        <LayoutDisplay Layout="@typeof(MainLayout)">
            <h1>Not found</h1>
            Sorry, there's nothing here.
        </LayoutDisplay>
    </NotFoundContent>
    <FoundContent>
        <AttributeAuthorizeView Page="@routeContext.Page">
            <Authorized>
                <LayoutDisplay Page="@routeContext.Page" PageParameters="@routeContext.Parameters" />            
            </Authorized>
            <NotAuthorized>
                <LayoutDisplay Layout="@typeof(MainLayout)">
                    <h1>Access denied</h1>
                    Go away.
                </LayoutDisplay>
            </NotAuthorized>
            <Authorizing>
                <LayoutDisplay Layout="@typeof(MainLayout)">
                    <h1>Please wait</h1>
                    Checking authorization...
                </LayoutDisplay>
            </Authorizing>
        </AttributeAuthorizeView>
    </FoundContent>
</Router>

This would completely nuke issue #10445 in the most comprehensive manner.

One drawback is the amount of repetition of specifying <LayoutDisplay Layout="@typeof(MainLayout)"> here, but doing this does eliminate any need for a DefaultLayout concept, and completely decouples Router, AuthorizeView, and LayoutDisplay from each other. They would no longer need to have any awareness of each others' existence.

@rynowak @danroth27 @javiercn What's your gut response to this? Does it look too complicated, or do you think the flexibility warrants all the explicit code?

Plan

  • Implement LayoutView
  • Implement RouteView
  • Update Router to new factoring
  • Implement AuthorizeRouteView
  • Remove CascadingAuthenticationState No, keeping it.
  • Remove PageDisplay
  • Update E2E tests
  • Update project templates
@mkArtakMSFT
Copy link
Member

Discussed this offline. We don't have clear picture regarding this adding value or not.

@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label May 24, 2019
@rynowak rynowak reopened this Jul 25, 2019
@rynowak
Copy link
Member

rynowak commented Jul 25, 2019

Reactivating this based on API review discussions. This solve a bunch of hard layering problems that we think are worth taking on. We can think of a few practical design issues in this area that will help users as well, and decoupling all of these things is a good long term choice.

We're planning to move the authorization features to Microsoft.AspNetCore.Components.Authorization

We're planning to move the forms support to Microsoft.AspNetCore.Components.Forms.

We plan to drop Microsoft.JSInterop as a dependency from M.A.Components and it will no longer have any dependencies 👍

@rynowak rynowak added this to the 3.0.0-preview9 milestone Jul 25, 2019
@rynowak rynowak self-assigned this Jul 25, 2019
@javiercn
Copy link
Member

How does this affect #10445. Should we fold this into here?

@javiercn
Copy link
Member

If we are considering moving forms and authorization out of M.A.Components. Should we also consider move Routing support to Microsoft.AspnetCore.Components.Routing?
I think they are at the same "level" and would be worth considering. The main challenge would be the JS interop piece, but that would use standard swa infrastructure to be available from the package and we could do some magic to plug it in automatically.

  • Router/NavLink/Location could invoke some JSInterop to load the script for interception magically.

Thoughts?

@rynowak
Copy link
Member

rynowak commented Jul 26, 2019

If we are considering moving forms and authorization out of M.A.Components. Should we also consider move Routing support to Microsoft.AspNetCore.Components.Routing?

We were talking about this today, and we don't have a really clear picture of what advantage this provides. If you want to make a case for it - what types would move there and what are the advantages of this?

@SteveSandersonMS
Copy link
Member Author

Done in #12800

@SteveSandersonMS SteveSandersonMS added the Done This issue has been fixed label Aug 5, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants