diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bccb76dab..7fe9d067a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -57,6 +57,10 @@ of the following. ## Vue / Quasar +UI rules beyond this list (color roles, typography, spacing, motion, component +choice) live in `DESIGN.md`, which is authoritative. Consult it before building +or changing UI; the rules below are the subset worth flagging in review. + - Use Quasar components (`q-btn`, `q-dialog`, `q-banner`, etc.). Selects need `dense` + `options-dense`. - **Dialogs (`q-dialog`)**: must have an accessible name (`aria-labelledby` @@ -69,7 +73,13 @@ of the following. `role="alert"`; warning/info default to `role="status"`. Use `live="assertive"` only for direct user-action responses, not persistent state indicators. Razor pages use `q-banner` with accessible classes - (`bg-warning text-dark`, `role="status"` or `role="alert"`). + (`bg-warning text-dark`, `role="status"` or `role="alert"`) for these + in-flow banners. +- **Status messages, toast vs banner**: a banner is a persistent, in-flow + message tied to page state. Transient confirmation that an action completed + is a toast instead, in Razor and Vue alike: `showStatusNotification()`, or + `queueStatusNotification()` when the action redirects. Do not swap these for + `q-banner` or Quasar `Notify`, neither works here; see `DESIGN.md`. - **Button colors**: `primary` (Aggie Blue), `positive` (create), `negative` (delete), `info text-color="dark"` (tertiary), `warning text-color="dark"` (caution), `secondary`. diff --git a/DESIGN.md b/DESIGN.md index 0394d9e9f..a6210f794 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -341,7 +341,9 @@ Use **`StatusBadge`**, which wraps `q-badge` and does two things automatically: ### Banners -Use **`StatusBanner`** in Vue SPAs with `type="success|error|warning|info"`. Each type carries its own Material icon (`check_circle`, `error`, `warning`, `info`), a 12% tint background (15% for warning), a 0.25rem left border, and matching text color. Only `type="error"` is assertive (`role="alert"`) by default; everything else is polite (`role="status"`). Override with `live`: `live="assertive"` for a warning or info banner shown in direct response to a user action, `live="off"` for a decorative banner with no dynamic content. Do not reach for `type="warning"` to force an assertive announcement on a persistent state indicator. Banners are `rounded` with `inline-actions`, sit on `q-mb-md`, and accept an optional dismiss button. Razor pages use `q-banner` with accessible classes. Error surfaces outside `StatusBanner` use the shared `.error-surface` treatment so `GenericError` and expired-session dialogs match. +Use **`StatusBanner`** in Vue SPAs with `type="success|error|warning|info"`. Each type carries its own Material icon (`check_circle`, `error`, `warning`, `info`), a 12% tint background (15% for warning), a 0.25rem left border, and matching text color. Only `type="error"` is assertive (`role="alert"`) by default; everything else is polite (`role="status"`). Override with `live`: `live="assertive"` for a warning or info banner shown in direct response to a user action, `live="off"` for a decorative banner with no dynamic content. Do not reach for `type="warning"` to force an assertive announcement on a persistent state indicator. Banners are `rounded` with `inline-actions`, sit on `q-mb-md`, and accept an optional dismiss button. Razor pages use `q-banner` with accessible classes for these in-flow banners. Error surfaces outside `StatusBanner` use the shared `.error-surface` treatment so `GenericError` and expired-session dialogs match. + +**Banner or toast?** A banner is a persistent message that sits in the page flow and reflects page state: validation, warnings, empty and error states. Transient confirmation that an action just completed is the status toast (`.viper-status-notification`) instead, in Razor and Vue alike, via `showStatusNotification()`, or `queueStatusNotification()` when the action redirects and the message has to survive the navigation. Quasar's `Notify` is unusable in the Razor pages: the app mounts on ``, so Notify's teleport container falls outside Vue's reactive scope and messages are dropped silently. ### Cards and Containers diff --git a/eslint.config.mjs b/eslint.config.mjs index 48af0f47a..d5c1b7075 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -83,6 +83,8 @@ export default [ getItemFromStorage: "readonly", putItemInStorage: "readonly", showStatusNotification: "readonly", + queueStatusNotification: "readonly", + showQueuedStatusNotification: "readonly", Quasar: "readonly", }, }, diff --git a/test/Classes/LeftNavHighlightTests.cs b/test/Classes/LeftNavHighlightTests.cs new file mode 100644 index 000000000..d07b4cea4 --- /dev/null +++ b/test/Classes/LeftNavHighlightTests.cs @@ -0,0 +1,158 @@ +using Viper.Classes; + +namespace Viper.test.Classes +{ + public class LeftNavHighlightTests + { + // Stands in for IUrlHelper.Content. The prefix mimics the "/2" PathBase used on + // TEST and PROD, so resolution is exercised the way it behaves off the app root. + private const string PathBase = "/2"; + private static string ResolveAppPath(string url) => PathBase + url.TrimStart('~'); + + private static (int ActiveIndex, int SecondaryActiveIndex) FindActive(List items, string requestPath) + => LeftNavHighlight.FindActive(items, requestPath, ResolveAppPath); + + private static NavMenuItem Link(string url) => new() { MenuItemText = url, MenuItemURL = url }; + + [Fact] + public void FindActive_RelativeLinkForCurrentPage_IsPrimary() + { + var items = new List { Link("Rolelist"), Link("RoleTemplateList") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleTemplateList"); + + Assert.Equal(1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_ChildPage_HighlightsParentItem() + { + // RoleTemplateRoles has no nav entry, so the Role Templates item stays lit. + var items = new List + { + Link("Rolelist"), + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateRoles", "RoleTemplateApply" } } + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleTemplateRoles"); + + Assert.Equal(1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_ChildPageWithQueryString_HighlightsParentItem() + { + // The child page is always reached with ?roleTemplateId=, which is not part of the path. + var items = new List + { + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateApply?roleTemplateId=1" } } + }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleTemplateApply"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_UnrelatedPage_HighlightsNothing() + { + var items = new List + { + Link("Rolelist"), + new() { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList", ChildPageURLs = { "RoleTemplateRoles" } } + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/AuditTrail"); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_OnlyInstanceLinkMatches_IsPromotedToPrimary() + { + var items = new List { Link("~/raps/Viper/RoleList"), Link("~/raps/VMACS.VMTH/RoleList") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_PageAndInstanceLinkMatch_PageLinkIsPrimary() + { + var items = new List { Link("~/raps/Viper/Rolelist"), Link("Rolelist") }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/Rolelist"); + + Assert.Equal(1, active); + Assert.Equal(0, secondary); + } + + [Fact] + public void FindActive_MatchIsCaseInsensitiveAndIgnoresTrailingSlash() + { + var items = new List { Link("rolelist/") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_ExternalAndEmptyUrls_NeverMatch() + { + var items = new List + { + new() { MenuItemText = "Header", MenuItemURL = "" }, + Link("https://ucdavis.edu/2/raps/Viper/RoleList"), + Link("mailto:someone@ucdavis.edu") + }; + + var (active, secondary) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_UnresolvableInstanceLink_NeverMatches() + { + // IUrlHelper.Content is nullable, so a URL it cannot resolve must not be + // compared against the request path as if it had resolved to nothing. + var items = new List { Link("~/raps/Viper/RoleList") }; + + var (active, secondary) = LeftNavHighlight.FindActive(items, "/2/raps/Viper/RoleList", _ => null); + + Assert.Equal(-1, active); + Assert.Equal(-1, secondary); + } + + [Fact] + public void FindActive_RequestPathWithTrailingSlash_StillMatches() + { + var items = new List { Link("RoleList") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList/"); + + Assert.Equal(0, active); + } + + [Fact] + public void FindActive_RootRelativeUrl_MatchesWithoutBasePath() + { + // Regression guard, and not as trivial as it looks: Uri parses a leading-slash + // path as an absolute file:// URI on Unix but not on Windows, so ordering the + // absolute-URL check before the root-relative one passes on a dev machine and + // fails on the Linux CI runner. + var items = new List { Link("/2/raps/Viper/RoleList") }; + + var (active, _) = FindActive(items, "/2/raps/Viper/RoleList"); + + Assert.Equal(0, active); + } + } +} diff --git a/web/Areas/RAPS/Controllers/RAPSController.cs b/web/Areas/RAPS/Controllers/RAPSController.cs index bd1d7d515..aaaf3c877 100644 --- a/web/Areas/RAPS/Controllers/RAPSController.cs +++ b/web/Areas/RAPS/Controllers/RAPSController.cs @@ -128,7 +128,13 @@ public async Task Nav(int? roleId, int? permissionId, string? memberId, } if (_securityService.IsAllowedTo("ViewRoles", instance)) { - nav.Add(new NavMenuItem { MenuItemText = "Role Templates", MenuItemURL = "RoleTemplateList" }); + nav.Add(new NavMenuItem + { + MenuItemText = "Role Templates", + MenuItemURL = "RoleTemplateList", + // These pages are reached from the template listing and have no nav entry of their own + ChildPageURLs = { "RoleTemplateRoles", "RoleTemplateApply" } + }); } if (selectedRole != null && RAPSSecurityService.RoleBelongsToInstance(instance, selectedRole)) { diff --git a/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml b/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml index 2b1818408..dbbd907d0 100644 --- a/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml +++ b/web/Areas/RAPS/Views/Roles/DelegateRoles.cshtml @@ -37,28 +37,29 @@ methods: { loadRoles: async function() { this.rolesLoaded = false - var childRoles = await viperFetch(this, "Roles/ControlledBy/" + + this.urlParams.get("roleId")) - var allRoles = await viperFetch(this, "Roles?Application=0") + this.loadingRoles = true + const [childRoles, allRoles] = await Promise.all([ + viperFetch(this, "Roles/ControlledBy/" + this.urlParams.get("roleId")), + viperFetch(this, "Roles?Application=0") + ]) + this.loadingRoles = false // A failed or still-pending read must not look like "nothing is // selected": submitting that would PUT an empty list and wipe the // existing delegations. Stay disabled until both reads succeed. if (childRoles === undefined || allRoles === undefined) { return } + const childRoleIds = new Set(childRoles.map(cr => cr.roleId)) this.selectedRoles = childRoles - this.roles = childRoles.concat( - allRoles.filter(r => childRoles.find(cr => cr.roleId === r.roleId) === undefined)) + this.roles = childRoles.concat(allRoles.filter(r => !childRoleIds.has(r.roleId))) this.rolesLoaded = true }, submitChanges: async function() { if (!this.rolesLoaded) { return } - var roleIds = this.selectedRoles.reduce( (result, role) => { - result.push(role.roleId) - return result - }, []) - viperFetch(this, + const roleIds = this.selectedRoles.map(role => role.roleId) + const result = await viperFetch(this, "Roles/ControlledBy/" + this.urlParams.get("roleId"), { method: "PUT", @@ -67,12 +68,14 @@ }, [this.loadRoles] ) + if (result !== undefined) { + showStatusNotification("Delegated roles updated") + } } }, async mounted() { await this.loadRoles() this.role = (await viperFetch(this, "Roles/" + this.urlParams.get("roleId"))) ?? {} - } }) diff --git a/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml b/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml index 89f742c15..10002d5b8 100644 --- a/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml +++ b/web/Areas/RAPS/Views/Roles/TemplateRoles.cshtml @@ -12,7 +12,7 @@ :pagination="{rowsPerPage:0}" :loading="loadingRoles">