search

aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 870 forks source link

ActivePage declarations missing on scaffolded Identity UI #1866

Closed tstivers1990 closed 6 years ago

tstivers1990 commented 6 years ago

Some of the ActivePage declarations are missing on the default scaffolded Identity UI. This causes some pages not to show an active item in the relevant nav section.

Some pages also use an ActivePage declaration that doesn't have a corresponding item in the navigation section, resulting in those pages not showing an active navigation item.

I've created a pull request that adds the missing ActivePage declarations, and changes the ones that don't have a corresponding nav element to use an ActivePage declaration with the most relevant nav element. For example, the DeletePersonalData page was using ViewData["ActivePage"] = ManageNavPages.DeletePersonalData; There is no nav element for Delete Personal Data. I've changed it to use ViewData["ActivePage"] = ManageNavPages.PersonalData; instead, as there is a nav element for Personal Data, and that is the most relevant nav element for the page.

Any methods and variables within ManageNavPages that are no longer used after this changed were removed. For example, the ManageNavPages.DeletePersonalData variable is no longer used after the change, and neither is the ManageNavPages.DeletePeronalDataNavClass method. Therefore, both of them are removed.

I've also made the PageNavClass method private, as it is never accessed outside of its class and, based on the class' design, should only be referenced by public methods in the class, and not directly outside of the class.

PR: #1838

javiercn commented 6 years ago

@tstivers1990 Thanks for filing the issue. It's clearer to me now. I've commented on the PR https://github.com/aspnet/Identity/pull/1838. The changes look good, but we need to remove the breaking changes as we want to take this into a patch.

@mkArtakMSFT This is a patch candidate.

mkArtakMSFT commented 6 years ago

Hey @vijayrkn, this will require also a fix in Scaffolder. I'll take this one to the Shiproom.

vijayrkn commented 6 years ago

@seancpeters - Can you please make the corresponding scaffolding change?