dotnet / Scaffolding

Code generators to speed up development.
MIT License
624 stars 223 forks source link

Views/Shared/_Layout.cshtml not updated when scaffolding identity in 2.2 release #931

Open chrisckc opened 5 years ago

chrisckc commented 5 years ago

Steps to reproduce:

Install release version of SDK 2.2 on Mac

dotnet --version

2.2.100

dotnet new  mvc

Then init a git repo with suitable .gitignore and commit everything so we can easily see what was changed.

Add the scaffolded identity files:

dotnet tool update --global dotnet-aspnet-codegenerator

Tool 'dotnet-aspnet-codegenerator' was successfully updated from version '2.1.6' to version '2.2.0'.

Add this to csproj file as requested when attempting to run the generator: <PackageReference Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="2.2" PrivateAssets="All" />

dotnet restore

Run the code generator:

dotnet-aspnet-codegenerator identity

Building project ... Finding the generator 'identity'... Running the generator 'identity'... RunTime 00:00:12.66

Run the app and notice there is no "Login" link in the nav bar.

Checking the contents of "Views/Shared/_Layout.cshtml" shows that the file "Views/Shared/_LoginPartial.cshtml" which was added by the code generator is not referenced and git shows no changes to the file.

Despite the lack of warning about updating existing file, run the generator again with the '--force' option:

Run the code generator with --force:

dotnet-aspnet-codegenerator identity --force

No difference.

Running:

dotnet-aspnet-codegenerator -h

shows there the force option is no longer there?

Expected behavior:

It should not generate broken scaffolding, the "Login" link should be there, it should work as it did previously in 2.1.

Actual behavior:

Code generator generates broken scaffolding for identity

Workaround:

This is easily be fixed by adding the partial to _Layout.cshtml, right after the navbar markup:

<div class="navbar-collapse collapse d-sm-inline-flex flex-sm-row-reverse">
    ..............                
</div>
<partial name="_LoginPartial" />

Seems there is also another issue with identity scaffolding, which i think also existed in 2.1 https://github.com/aspnet/Scaffolding/issues/884

chrisckc commented 5 years ago

To make it look correct on small screen size, the partial needs to be inside the navbar instead so it becomes part of the hamburger menu:

<div class="navbar-collapse collapse d-sm-inline-flex flex-sm-row-reverse">
    <ul class="navbar-nav flex-grow-1">
     ........
    </ul>
    <partial name="_LoginPartial" />
</div>

But this then doesn't not look correct on normal screen size as the links are no longer on the right hand side of the nav menu, suggesting there is also some css updates missing in the generated files.

chrisckc commented 5 years ago

@blowdart After logging in and looking at the Identity UI, i noticed that the top navbar style does not match the new updated style of the MVC app as it's using navbar dark mode as it was in 2.1. The nav menu is very basic and does not properly stack vertically, the profile and logout links in the top nav bar are also out of alignment.

I then compared the scaffolded identity UI to the non-scaffolded version created using:

dotnet new mvc --auth Individual

The non-scaffolded UI is fine, the Login link is there and once logged in, the UI styles match and look more like they should do.

BTW. It seems that the intended place to put "_LoginPartial" is here:

<div class="navbar-collapse collapse d-sm-inline-flex flex-sm-row-reverse">
    <ul class="navbar-nav">
        <partial name="_LoginPartial" />
    </ul>
    <ul class="navbar-nav flex-grow-1">
        <li class="nav-item">
            <a class="nav-link text-dark" asp-area="" asp-controller="Home" asp-action="Index">Home</a>
        </li>
        <li class="nav-item">
            <a class="nav-link text-dark" asp-area="" asp-controller="Home" asp-action="Privacy">Privacy</a>
        </li>
    </ul>
</div>

To see screenshots, have a look at my blog post on the Identity template changes in 2.2 in which i have also detailed the issues that i encountered: https://www.chrisclaxton.me.uk/chris-claxtons-blog/aspdotnetcore-22-mvc-identity-templates

chrisckc commented 5 years ago

Tried adding .AddDefaultUI(UIFramework.Bootstrap4) from the Identity startup code obtained from a project generated with dotnet new mvc --auth Individual which looks like this

services.AddDbContext<ApplicationDbContext>(options =>
    options.UseSqlite(
        Configuration.GetConnectionString("DefaultConnection")));
services.AddDefaultIdentity<IdentityUser>()
    .AddDefaultUI(UIFramework.Bootstrap4)
    .AddEntityFrameworkStores<ApplicationDbContext>();

The UI now looks correct, and i am still able to override the UI by editing the scaffolded files, so there must be some merging going on somewhere. It still needs _Layout.cshtml to be updated by adding _LoginPartial.cshtml.

EDIT: Adding .AddDefaultUI(UIFramework.Bootstrap4) to the Default Identity configuration introduces an issue with footer on the "/Identity/Account/Manage/" pages which now looks bad due to the addition of "overflow: scroll;"

It seems that the css for the "/Identity/Account/Manage/" pages is now overridden, the following additional css is used in site.css:

.container {
  max-width: 960px;
}

.pricing-header {
  max-width: 700px;
}

pricing-header does not appear to be used anywhere

The following footer css is replaced with this:

.footer {
  position: absolute;
  bottom: 0;
  width: 100%;
  overflow: scroll;
  white-space: nowrap;
  /* Set the fixed height of the footer here */
  height: 60px;
  line-height: 60px; /* Vertically center the text there */
}

The difference is the addition of "overflow: scroll;" which makes the footer look bad on the "/Identity/Account/Manage/" pages.

chrisckc commented 5 years ago

Digging further it seems that the best solution is to edit the file: "/Areas/Identity/Pages/Account/Manage/_Layout.cshtml"

The top of the file contains the following layout reference:

@{ 
    Layout = "/Areas/Identity/Pages/_Layout.cshtml";
}

That file does not exist in the scaffolded files, so must be provided magically, see: https://github.com/aspnet/Scaffolding/issues/884 Unless .AddDefaultUI(UIFramework.Bootstrap4) is used in the Default Identity configuration, the Bootstrap v3 copy of the missing file is provided.

The best solution is to simply replace the reference the app's main layout file instead

@{ 
    Layout = "/Views/Shared/_Layout.cshtml";
}

this is the same as what is specified in the "/Areas/Identity/Pages/_ViewStart.cshtml" file.

This should be clearly documented somewhere.

joyahmed commented 5 years ago

I had done everything advised here but nothing worked. Finally had overwritten bootstrap css and js files manually and it worked

Rick-Anderson commented 4 years ago

Duplicate of #1225