Feature task 3634 tdei theme#54
Conversation
Introduce TDEI branding: add tdei-logo.png, import Lato/Open Sans/Montserrat fonts, update theme primary color to #32006e, and add extensive utility CSS (color variables, button/link styles, layout helpers, accessibility focus styles) in main.scss. Replace many Bootstrap button/variant uses across components/pages with new tdei-* classes (tdei-primary-button, tdei-secondary-button, maroon-bg, tdei-primary-link) and adjust dialog button classes/confirm styles to match the new design system. These changes unify styling and apply the updated TDEI visual identity across the app.
Add new asset (osw-type.svg) and update global styles and pages to improve layout and mobile UX. Main changes: - components/AppNavbar.vue: Rework navbar into a responsive, sticky header with a mobile side menu and updated user dropdown. Adds state and lifecycle hooks to manage mobile menu visibility, accessibility attributes, and dedicated mobile interactions (close button, backdrop, logout flow). Extensive SCSS added to style the navbar, side menu, and responsive behaviors. - pages/index.vue: Make the primary CTA conditional on authentication (redirect to dashboard when signed in), import auth in the script, and update hero styling to use CSS variables and refined colors/hover states. - assets/scss/main.scss: Update font imports (remove Lato, ensure Open Sans and Montserrat are imported). - assets/img/osw-type.svg: Add new SVG asset. These changes are intended to improve navigation on small screens, unify header styling, and make the landing CTA reflect user authentication state.
Revamp dashboard workspace list and toolbar styling: remove duplicate Montserrat font import; rename toolbar button classes to tdei-toolbar-primary-button and add scoped styles for toolbar buttons and grouped button radii. Overhaul WorkspaceItem into a card-style layout (new markup, styles, responsive adjustments), show workspace ID, format type/status labels, and simplify class logic. Update dashboard page to add a page header subtitle, adjust Project Group label styling, wrap workspace list in a template, and add getLastProjectGroupName + selectedProjectGroupName computed to derive the displayed project group name from sessionStorage or project groups.
Add flex and pathways SVG icons and update WorkspaceItem.vue to select the correct icon based on workspace.type. Imports osw, flex, and pathways assets, introduces a computed typeIconSrc that maps types to their icons (falls back to osw), and replaces the hardcoded img src with a bound :src. Enables showing the appropriate icon for different workspace types.
Refactor the homepage template and SCSS to introduce a new hero layout and visual update. Replaced .jumbo-content with .hero-shell and .hero-copy, moved CTAs into a .hero-actions container (primary Nuxt link + external "Learn more"), and kept the auth-based dashboard/signin routing. Reworked background to use layered gradients over the existing bg.webp, added a decorative overlay, improved typography, spacing, and app-logo styling, and refined responsive behavior across breakpoints (lg, md, sm). These changes modernize the landing hero, improve readability and responsiveness, and polish CTA styles and spacing.
SCSS: Remove unnecessary !important from primary button rules, add proper :focus styles (box-shadow) for accessibility, and introduce .tdei-outline-cyan-button (replacing .tdei-button2) with hover/focus/disabled states. Unify radio focus styling and update shadow color to match brand. Minor CSS cleanup and spacing adjustments. Components: remove d-block from the Apply button in FilterDropdown. Pages: simplify selectedProjectGroupName logic in dashboard.vue (drop getLastProjectGroupName fallback) and reduce hero-primary-action min-height in index.vue.
Add an accessible alt attribute for workspace type images and adjust SCSS variables and styles. In WorkspaceItem.vue the img alt is now dynamic (uses formatTypeLabel) to improve accessibility. In assets/scss/main.scss: remove duplicate --primary-color-dark, add --tdei-cyan, change .jsonContent border to use --secondary-color, remove custom radio focus styles, and adjust .column-style flex/margins. Also restore/move text-transform: capitalize for .workspaceStatus to the stronger selector to keep visual consistency.
Add focus management and keyboard support for the mobile side menu: add refs for the toggle button and side menu, focus the close button when opened and restore focus to the toggle when closed, trap Tab/Shift+Tab inside the menu, handle Escape to close, and add aria-modal="true". Attach/detach keydown listener on mount/unmount. Minor style updates: introduce --primary-color-dark and small SCSS cleanup. Remove an unused getLastProjectGroupName() helper from dashboard.vue. Add rel="noopener noreferrer" to an external link for security.
SCSS: Update .tdei-rounded-button to .btn.tdei-rounded-button, remove unnecessary !important rules, nest hover/active state and use --primary-color-dark for hover/border. Also fix stray brace and tighten selector scope. AppNavbar.vue: Use focusable.at(0) and .at(-1) for first/last element and add a null check to avoid runtime errors when focusable is empty. pages/dashboard.vue: Render the "in <project group>" phrase only when a project group name exists and change the computed fallback from a literal 'project group' to null to avoid showing a placeholder when none is selected.
Overhaul the sign-in experience and adjust several layout components. Add a new background image asset and replace the signin page to render the new SigninForm component with a centered card, background, and responsive styles. Revamp SigninForm: new header, show/hide password toggle, improved loading/error states, and extensive SCSS for desktop/mobile. Tidy up AppNavbar templates and mobile menu markup (focus handling/focus trap fixes and markup formatting). Minor tweaks: AppSpinner markup, default layout nuxt-loading-indicator props, and refactor dashboard into a split layout with scrollable workspace list and sticky details panel.
Bind aria-modal to return 'true' or 'false' based on mobileMenuOpen/isMobileView to fix accessibility attribute type. Strengthen SigninForm error handling by adding a fetch-error type guard, parsing 400 responses for validation errors, and providing specific messages for 401 and 404 cases; also normalize other errors to strings. Add missing 'btn' class to the Close button in JoinDialog for consistent styling.
Add role="dialog" to the side menu element in AppNavbar.vue to improve accessibility and complement existing ARIA attributes. Remove an explicit color rule from .team-members-btn in teams/Item.vue to stop overriding the global body color and keep link styling consistent.
Remove external @import URL lines from assets/scss/main.scss and add the @nuxtjs/google-fonts module and configuration in nuxt.config.ts (register Open Sans and Montserrat, set display: 'swap', download: true). Also add @nuxtjs/google-fonts to devDependencies in package.json. Centralizes font loading, enables local font download/optimization, and avoids inline external CSS imports.
Add a long CSS transition to neutralize Chrome's autofill background, preventing the yellow/blue autofill styling from showing. Replace the AppNavbar side menu <aside> with a <div> to match the component structure. Scope SigninForm styles (add scoped) and remove the preset input background so autofill styling is handled consistently. Add an explicit Workspace type import and annotate selectWorkspace(workspace) with the Workspace type.
|
Warning Review limit reached
More reviews will be available in 45 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR delivers a comprehensive design system refresh, converting the application from Bootstrap variants to custom TDEI-branded styling, updating the primary theme color from purple ( ChangesDesign System & UI Styling Refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR contains moderate complexity with changes spread across the design foundation (SCSS variables, utilities), multiple component refactors (navbar, forms, dashboard), and widespread button/modal styling updates applied consistently across 20+ files. While the button styling changes are repetitive and follow a single pattern, the navbar redesign introduces new responsive logic, authentication flows have stricter type guards, and layout restructures (dashboard, landing page, sign-in) require careful validation of responsive behavior and component integration. Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/scss/main.scss`:
- Around line 9-10: The two CSS `@import` directives for
"maplibre-gl/dist/maplibre-gl.css" and "vue3-toastify/dist/index.css" are placed
after the input:-webkit-autofill rule, causing the
no-invalid-position-at-import-rule lint error; fix this by moving both `@import`
lines to the very top of assets/scss/main.scss so they appear before any style
rules (specifically before the input:-webkit-autofill selector and its block),
preserving their exact strings and order.
In `@components/AppNavbar.vue`:
- Around line 318-324: The CSS currently removes the keyboard focus indicator
for the mobile nav toggler (box-shadow: none) and only defines hover styles for
drawer controls, which hides focus for keyboard users; restore a visible focus
indicator by replacing the "box-shadow: none" rule on the toggler with a focused
style (use :focus and :focus-visible) that applies a visible outline or
box-shadow (e.g. using a --focus-ring-color and matching border-radius) and add
equivalent :focus/:focus-visible rules for the drawer controls so they show the
same focus ring as hover (but distinct for keyboard focus), ensuring you target
the same selectors used for the toggler and drawer controls in AppNavbar.vue so
keyboard users can track focus.
- Around line 65-74: The dropdown toggle currently nests a native <button>
inside the b-dropdown `#button-content` which breaks toggle semantics; replace the
inner interactive element (class "userProfile" button) with a non-interactive
wrapper (e.g., div or span) and move any click/keyboard handling to the
dropdown/toggle itself so the dropdown controls activation; ensure the wrapper
keeps the same classes/visual markup (class "userProfile") and accessible
labeling (use aria attributes on the toggle if needed) to preserve appearance
and accessibility.
In `@components/dashboard/WorkspaceItem.vue`:
- Around line 15-20: The role chip currently only handles 'lead' and 'validator'
and leaves 'contributor' or undefined blank; update the template in
WorkspaceItem.vue to add a v-else fallback that renders a span with class
"workspaceStatus" showing ROLE_LABELS.member (treat undefined the same as
'contributor'). Ensure you reference the existing workspace.role checks and
ROLE_LABELS constant so the new fallback collapses contributor/undefined into
the Member label.
In `@components/SigninForm.vue`:
- Around line 23-27: In SigninForm.vue, add autocomplete attributes to the
credential inputs so browsers/password managers work correctly: set
autocomplete="username" on the input using v-model="username" (id="username")
and set autocomplete="current-password" on the input using v-model="password"
(the password field around lines 43-47); ensure attributes are added to the same
element nodes that currently have class "form-control signin-input" and aria
attributes.
- Around line 107-113: The 400 error branch currently calls await
e.response.json() which can throw if the response body isn't valid JSON; wrap
the JSON parse in a safe guard (e.g., inspect
e.response.headers.get('content-type') for application/json and/or use a
try/catch around await e.response.json()) and fall back to a generic message
when parsing fails so error.value still gets set; update the logic around the
existing body variable and the error.value assignment in the SigninForm.vue
catch block to use the parsed body only when parse succeeds and otherwise set a
sensible fallback error.
In `@pages/signin.vue`:
- Around line 12-14: The redirect currently calls navigateTo('/dashboard')
without awaiting its promise; update the branch that checks tdeiAuth.complete to
await (or return) the navigateTo call so Nuxt handles the navigation
immediately—e.g., inside the script setup where tdeiAuth.complete is checked,
replace navigateTo('/dashboard') with await navigateTo('/dashboard') (or return
await navigateTo('/dashboard')) so the promise is not ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 784db07f-7b18-4614-9663-5037757aebfd
⛔ Files ignored due to path filters (5)
assets/img/bg_login.pngis excluded by!**/*.pngassets/img/flex-type.svgis excluded by!**/*.svgassets/img/osw-type.svgis excluded by!**/*.svgassets/img/pathways-type.svgis excluded by!**/*.svgassets/img/tdei-logo.pngis excluded by!**/*.png
📒 Files selected for processing (35)
assets/scss/main.scssassets/scss/theme.scsscomponents/AppLogo.vuecomponents/AppNavbar.vuecomponents/AppSpinner.vuecomponents/DatasetTypeRadio.vuecomponents/SigninForm.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/FilterDropdown.vuecomponents/review/Toolbar.vuecomponents/settings/panel/Apps.vuecomponents/settings/panel/Delete.vuecomponents/settings/panel/General.vuecomponents/settings/panel/Imagery.vuecomponents/settings/teams/JoinDialog.vuecomponents/teams/Item.vuecomponents/teams/JoinDialog.vuecomponents/teams/MembersDialog.vuecomponents/teams/SettingsDialog.vuelayouts/default.vuenuxt.config.tspackage.jsonpages/dashboard.vuepages/help.vuepages/index.vuepages/signin.vuepages/workspace/[id]/export/index.vuepages/workspace/[id]/export/tdei.vuepages/workspace/[id]/settings/teams/index.vuepages/workspace/[id]/teams/[teamId].vuepages/workspace/create/blank.vuepages/workspace/create/file.vuepages/workspace/create/index.vuepages/workspace/create/tdei.vue
| @import "maplibre-gl/dist/maplibre-gl.css"; | ||
| @import "vue3-toastify/dist/index.css"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether any `@import` appears after a non-import rule in main.scss
set -euo pipefail
FILE="$(fd '^main\.scss$' -a | head -n1)"
echo "Checking: $FILE"
awk '
BEGIN { seen_non_import=0; bad=0 }
{
line=$0
trimmed=line
sub(/^[[:space:]]+/, "", trimmed)
if (trimmed == "" || trimmed ~ /^\/\// || trimmed ~ /^\/\*/) next
if (trimmed ~ /^`@import`[[:space:]]+/) {
if (seen_non_import) {
printf("Invalid import position at line %d: %s\n", NR, line)
bad=1
}
next
}
# treat other top-level statements as non-import
seen_non_import=1
}
END {
if (bad) exit 1
print "OK: no invalid `@import` positions detected."
}
' "$FILE"Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="assets/scss/main.scss"
echo "== $FILE =="
nl -ba "$FILE" | sed -n '1,25p'Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="assets/scss/main.scss"
echo "== $FILE =="
cat -n "$FILE" | sed -n '1,25p'Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 1043
Move CSS @import directives to the top of assets/scss/main.scss (before style rules)
@import statements at lines 9-10 come after the input:-webkit-autofill rule block (lines 4-8), which triggers no-invalid-position-at-import-rule and can fail linting.
💡 Proposed fix
`@import` "theme.scss";
`@import` "bootstrap/scss/bootstrap.scss";
+@import "maplibre-gl/dist/maplibre-gl.css";
+@import "vue3-toastify/dist/index.css";
input:-webkit-autofill,
input:-webkit-autofill:hover,
input:-webkit-autofill:focus {
transition: background-color 600000s, color 600000s;
}
-@import "maplibre-gl/dist/maplibre-gl.css";
-@import "vue3-toastify/dist/index.css";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @import "maplibre-gl/dist/maplibre-gl.css"; | |
| @import "vue3-toastify/dist/index.css"; | |
| `@import` "theme.scss"; | |
| `@import` "bootstrap/scss/bootstrap.scss"; | |
| `@import` "maplibre-gl/dist/maplibre-gl.css"; | |
| `@import` "vue3-toastify/dist/index.css"; | |
| input:-webkit-autofill, | |
| input:-webkit-autofill:hover, | |
| input:-webkit-autofill:focus { | |
| transition: background-color 600000s, color 600000s; | |
| } |
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 9-9: Invalid position for @import rule (no-invalid-position-at-import-rule)
(no-invalid-position-at-import-rule)
[error] 10-10: Invalid position for @import rule (no-invalid-position-at-import-rule)
(no-invalid-position-at-import-rule)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/scss/main.scss` around lines 9 - 10, The two CSS `@import` directives
for "maplibre-gl/dist/maplibre-gl.css" and "vue3-toastify/dist/index.css" are
placed after the input:-webkit-autofill rule, causing the
no-invalid-position-at-import-rule lint error; fix this by moving both `@import`
lines to the very top of assets/scss/main.scss so they appear before any style
rules (specifically before the input:-webkit-autofill selector and its block),
preserving their exact strings and order.
| <template #button-content> | ||
| <button class="userProfile"> | ||
| <div>{{ auth.displayName }}</div> | ||
| <app-icon | ||
| variant="account_circle" | ||
| size="32" | ||
| no-margin | ||
| /> | ||
| </button> | ||
| </template> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In bootstrap-vue-next b-dropdown, is it valid to place a native <button> inside the #button-content slot, or should slot content be non-interactive because the component already renders the toggle button?
💡 Result:
Yes, it’s valid to put a native inside b-dropdown’s #button-content slot, because the BootstrapVueNext docs explicitly state that the button-content slot allows “basic HTML and icons in the button content” (i.e., it isn’t limited to non-interactive markup) [1]. However, for correct behavior and accessibility, you generally shouldn’t place another interactive control that the user can click to trigger its own separate action inside the dropdown-toggle button, because the entire header is already the control that toggles the dropdown. In practice: keep the slot content simple (text/icons) or ensure any nested interactive element does not interfere with the dropdown toggle’s click/keyboard handling. Evidence from the component docs: the #button-content slot is provided specifically to customize the dropdown toggle’s content with HTML/icons, and is not documented as “non-interactive only” [1].
Citations:
Avoid nested interactive semantics in b-dropdown #button-content (accessibility/behavior risk).
b-dropdown’s #button-content slot supports arbitrary HTML/icons (so nesting isn’t inherently “invalid markup”), but placing a native nested button inside the dropdown toggle content can interfere with the toggle’s click/focus/keyboard semantics and confuse screen readers. Prefer a non-interactive wrapper.
Suggested fix
<template `#button-content`>
- <button class="userProfile">
+ <span class="userProfile">
<div>{{ auth.displayName }}</div>
<app-icon
variant="account_circle"
size="32"
no-margin
/>
- </button>
+ </span>
</template>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <template #button-content> | |
| <button class="userProfile"> | |
| <div>{{ auth.displayName }}</div> | |
| <app-icon | |
| variant="account_circle" | |
| size="32" | |
| no-margin | |
| /> | |
| </button> | |
| </template> | |
| <template `#button-content`> | |
| <span class="userProfile"> | |
| <div>{{ auth.displayName }}</div> | |
| <app-icon | |
| variant="account_circle" | |
| size="32" | |
| no-margin | |
| /> | |
| </span> | |
| </template> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/AppNavbar.vue` around lines 65 - 74, The dropdown toggle currently
nests a native <button> inside the b-dropdown `#button-content` which breaks
toggle semantics; replace the inner interactive element (class "userProfile"
button) with a non-interactive wrapper (e.g., div or span) and move any
click/keyboard handling to the dropdown/toggle itself so the dropdown controls
activation; ensure the wrapper keeps the same classes/visual markup (class
"userProfile") and accessible labeling (use aria attributes on the toggle if
needed) to preserve appearance and accessibility.
| &:focus { | ||
| box-shadow: none; | ||
| } | ||
|
|
||
| &:hover { | ||
| background-color: var(--primary-color-dark); | ||
| } |
There was a problem hiding this comment.
Restore visible keyboard focus styles for mobile nav controls.
Lines 318-320 remove the toggler focus indicator (box-shadow: none) without a replacement, and drawer controls only define hover styles. Keyboard users can lose track of focus in the menu flow.
Suggested fix
.mobileMenuIcon {
padding: 5px 8px;
border: none;
- &:focus {
- box-shadow: none;
- }
+ &:focus-visible {
+ outline: 2px solid `#ffffff`;
+ outline-offset: 2px;
+ box-shadow: none;
+ }
&:hover {
background-color: var(--primary-color-dark);
}
}
+ .sideMenuClose:focus-visible,
+ .sideMenuLink:focus-visible,
+ .sideMenuButton:focus-visible {
+ outline: 2px solid var(--primary-color);
+ outline-offset: -2px;
+ }Also applies to: 447-452, 460-481
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/AppNavbar.vue` around lines 318 - 324, The CSS currently removes
the keyboard focus indicator for the mobile nav toggler (box-shadow: none) and
only defines hover styles for drawer controls, which hides focus for keyboard
users; restore a visible focus indicator by replacing the "box-shadow: none"
rule on the toggler with a focused style (use :focus and :focus-visible) that
applies a visible outline or box-shadow (e.g. using a --focus-ring-color and
matching border-radius) and add equivalent :focus/:focus-visible rules for the
drawer controls so they show the same focus ring as hover (but distinct for
keyboard focus), ensuring you target the same selectors used for the toggler and
drawer controls in AppNavbar.vue so keyboard users can track focus.
| id="username" | ||
| v-model="username" | ||
| class="form-control signin-input" | ||
| aria-describedby="usernameHelp" | ||
| > |
There was a problem hiding this comment.
Add autocomplete hints for credential fields.
Use autocomplete="username" and autocomplete="current-password" so browsers/password managers behave correctly.
Suggested fix
<input
id="username"
v-model="username"
class="form-control signin-input"
aria-describedby="usernameHelp"
+ autocomplete="username"
>
...
<input
id="password"
v-model="password"
:type="showPassword ? 'text' : 'password'"
class="form-control signin-input password-input"
+ autocomplete="current-password"
>Also applies to: 43-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/SigninForm.vue` around lines 23 - 27, In SigninForm.vue, add
autocomplete attributes to the credential inputs so browsers/password managers
work correctly: set autocomplete="username" on the input using
v-model="username" (id="username") and set autocomplete="current-password" on
the input using v-model="password" (the password field around lines 43-47);
ensure attributes are added to the same element nodes that currently have class
"form-control signin-input" and aria attributes.
| if (e.response.status === 400) { | ||
| const body = await e.response.json() as { errors?: string[] } | ||
|
|
||
| if (body.errors && body.errors.length > 0) { | ||
| error.value = 'Error: ' + body.errors[0] | ||
| return | ||
| } |
There was a problem hiding this comment.
Guard JSON parsing in the 400 error branch.
If a 400 response is not JSON, await e.response.json() throws inside the catch block and bypasses your user-facing fallback error handling.
Suggested fix
if (e.response.status === 400) {
- const body = await e.response.json() as { errors?: string[] }
-
- if (body.errors && body.errors.length > 0) {
- error.value = 'Error: ' + body.errors[0]
- return
- }
+ try {
+ const body = await e.response.json() as { errors?: string[] }
+ if (body.errors && body.errors.length > 0) {
+ error.value = 'Error: ' + body.errors[0]
+ return
+ }
+ }
+ catch {
+ // fall through to generic message below
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (e.response.status === 400) { | |
| const body = await e.response.json() as { errors?: string[] } | |
| if (body.errors && body.errors.length > 0) { | |
| error.value = 'Error: ' + body.errors[0] | |
| return | |
| } | |
| if (e.response.status === 400) { | |
| try { | |
| const body = await e.response.json() as { errors?: string[] } | |
| if (body.errors && body.errors.length > 0) { | |
| error.value = 'Error: ' + body.errors[0] | |
| return | |
| } | |
| } | |
| catch { | |
| // fall through to generic message below | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/SigninForm.vue` around lines 107 - 113, The 400 error branch
currently calls await e.response.json() which can throw if the response body
isn't valid JSON; wrap the JSON parse in a safe guard (e.g., inspect
e.response.headers.get('content-type') for application/json and/or use a
try/catch around await e.response.json()) and fall back to a generic message
when parsing fails so error.value still gets set; update the logic around the
existing body variable and the error.value assignment in the SigninForm.vue
catch block to use the parsed body only when parse succeeds and otherwise set a
sensible fallback error.
| if (tdeiAuth.complete) { | ||
| navigateTo('/dashboard'); | ||
| navigateTo('/dashboard') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Nuxt 4 page <script setup>, what is the recommended pattern for immediate auth redirects: plain navigateTo(...) vs return/await navigateTo(...)?
💡 Result:
Recommended pattern in a Nuxt 4 page <script setup> for immediate auth redirects is to use await (or return) with navigateTo, i.e. await navigateTo('/login') and then prevent further work as needed. What to do 1) In <script setup>, do: - await navigateTo('/login') (or await navigateTo({ path: '/login' })). Nuxt’s navigateTo docs explicitly show await navigateTo(...) usage in <script setup> examples. [1] 2) Avoid “fire-and-forget” usage like navigateTo('/login') without awaiting/returning. Nuxt’s navigateTo docs explicitly state: “Make sure to always use await or return on result of navigateTo when calling it.” [2][1] Plain navigateTo(...) vs return/await navigateTo(...) - navigateTo(...) without awaiting/returning: not the recommended approach; Nuxt advises awaiting/returning the result to ensure the navigation outcome is properly handled. [2][1] - return navigateTo(...): generally discouraged/fragile in <script setup> if you’re trying to “terminate” execution. Nuxt issue discussions note that there’s no reliable way to terminate execution in the <script setup> block and that return navigateTo() was previously possible but has been deprecated; the recommended approach becomes handling redirects with await (or other control-flow patterns). [3] - await navigateTo(...): the documented and recommended way in <script setup> to perform the redirect. [1] Practical guidance for auth redirect in a page - Use middleware for route guarding when possible (so pages don’t render at all). Nuxt’s routing docs show returning navigateTo from route middleware for auth gating. [4] - If you must do it in the page itself, use await navigateTo(...) early in the script. In short: prefer await navigateTo(...) in a page <script setup>; don’t call navigateTo(...) without awaiting/returning; and avoid relying on return navigateTo(...) to stop the rest of <script setup> execution. [2][1][3]
Citations:
- 1: https://nuxt.com/docs/4.x/api/utils/navigate-to
- 2: https://nuxt-nuxt.mintlify.app/api/utils/navigate-to
- 3: transform
navigateToin<script setup>to provide an early return nuxt/nuxt#23698 - 4: https://nuxt.com/docs/4.x/getting-started/routing
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
# Show the relevant portion of pages/signin.vue
if [ -f "pages/signin.vue" ]; then
echo "---- pages/signin.vue (context) ----"
nl -ba pages/signin.vue | sed -n '1,80p'
else
echo "pages/signin.vue not found"
fd signin.vue -a . || true
fiRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 2282
🌐 Web query:
Nuxt 4 navigateTo in <script setup> "Make sure to always use await or return"
💡 Result:
In Nuxt 4, navigateTo returns a value/promise (depending on context), and the docs explicitly instruct: “Make sure to always use await or return on result of navigateTo when calling it.” [1] What this means in <script setup> 1) If you call navigateTo directly from your setup code, you should await it: await navigateTo('/search') [1] or await navigateTo({ path: '/search' }) [1] 2) If you are in an async function, you can also return the navigateTo call instead of awaiting it, as long as you “return its result” to keep the calling flow consistent (the docs’ wording covers both await and return) [1]. Route middleware special case If you call navigateTo inside defineNuxtRouteMiddleware, you must return it so the middleware execution flow works correctly. The docs show the correct form: return navigateTo('/search', { redirectCode: 301 }) [1] and the incorrect form (navigateTo executed but not returned): navigateTo('/search', { redirectCode: 301 }) return [1] Why you should do this The official docs warn that not awaiting/returning can lead to “unexpected behavior” in middleware, and they also provide concrete examples of calling navigateTo with await in <script setup> [1]. Note on “return navigateTo” vs “terminate setup execution” Nuxt maintainers have discussed that terminating execution inside <script setup> is not straightforward; using return navigateTo there can be deprecated/insufficient for aborting setup flow. A tracked Nuxt issue discusses the limitation and suggests patterns/workarounds, but the core, source-backed guidance for navigateTo usage is still to await or return its result when you call it [2].
Citations:
- 1: https://nuxt.com/docs/4.x/api/utils/navigate-to
- 2: transform
navigateToin<script setup>to provide an early return nuxt/nuxt#23698
🌐 Web query:
Nuxt 4 navigateTo return vs await guidance in pages
💡 Result:
In Nuxt 4, for programmatic navigation with navigateTo, you should either: 1) return navigateTo(...) from functions that use it (notably route middleware), or 2) await navigateTo(...) in async contexts where you just need to wait for the navigation to complete. Key guidance differences A) In route middleware: you must return Nuxt’s docs explicitly say that when using navigateTo within route middleware, you must return its result to ensure the middleware execution flow works correctly, and they show the non-working pattern where navigateTo(...) is called but not returned. [1] Example (middleware redirect): ts export default defineNuxtRouteMiddleware((to, from) => { if (to.path !== '/search') { return navigateTo('/search', { redirectCode: 301 }) } }) [1] B) In pages/components: prefer await for “do this after navigation” Nuxt’s navigateTo docs also show using await navigateTo('/search') (or awaiting a route object) in pages/components style usage. [1] Example (page/component): ts <script setup lang="ts"> await navigateTo({ path: '/search', query: { page: 1 } }) </script> [1] What “return vs await” really changes - return navigateTo(...) is mainly about control flow: it passes the navigation result back to the caller. This is required in middleware because the router/middleware runner needs the return value. [1] - await navigateTo(...) is about sequencing: it pauses your async code until navigation finishes/settles. The docs instruct to “always use await or return on result of navigateTo when calling it.” [1] C) Why “await but not return” can be problematic in middleware A Nuxt issue discusses middleware behavior and highlights that you need correct handling of navigateTo result in middleware contexts. [2] So: concise recommendation - Route middleware: use return navigateTo(...) (or return await navigateTo(...) if you want, but return is the important part). [1] - Pages/components (async handler): use await navigateTo(...) if you need to wait; otherwise, just ensure you use either await or return as recommended by the docs (don’t call it and ignore the result). [1]
Citations:
- 1: https://nuxt.com/docs/4.x/api/utils/navigate-to
- 2: can't return
navigateToafter await in route middleware (on server) nuxt/nuxt#13340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant portion of pages/signin.vue
if [ -f "pages/signin.vue" ]; then
echo "---- pages/signin.vue (context) ----"
nl -ba pages/signin.vue | sed -n '1,120p'
else
echo "pages/signin.vue not found"
fd -a -i "signin.vue" . || true
fiRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 178
🌐 Web query:
Nuxt 4 navigateTo in <script setup> "Make sure to always use await or return"
💡 Result:
In Nuxt 4, when you call navigateTo in <script setup>, you should “always use await or return” because navigateTo returns a value/promise that you need to handle to make the control-flow and navigation behavior correct. The official docs explicitly say: “Make sure to always use await or return on result of navigateTo when calling it.” [1] What to do in <script setup> 1) Use await inside an async function or top-level await in <script setup>: await navigateTo('/search') [1] 2) If your function returns the navigation result, return it: return navigateTo('/search') (Using await or return ensures you don’t ignore the navigation result.) [1] Why the rule matters (route middleware example) Nuxt’s docs also show that inside route middleware you must return navigateTo’s result; otherwise the redirect won’t integrate correctly with the middleware flow: return navigateTo('/search', { redirectCode: 301 }) // ✅ [1] navigateTo('/search', { redirectCode: 301 }); return // ❌ not returned [1] So the underlying principle is: don’t fire navigateTo and drop its return value; use await or return so Nuxt/your code can correctly observe/propagate the navigation result. [1] Notes on return type The docs describe navigateTo’s signature returning a Promise (among other union members), which is consistent with why awaiting/returning is recommended. [1]
Citations:
🌐 Web query:
Nuxt 4 navigateTo return vs await guidance in pages redirect
💡 Result:
In Nuxt 4, use either await or return from navigateTo, but the best guidance depends on where you call it. 1) In route middleware (defineNuxtRouteMiddleware): you must return navigateTo - Nuxt’s docs explicitly say that when using navigateTo within route middleware, you must return its result so the middleware execution flow works correctly [1]. - The middleware docs also list return values such as “return navigateTo('/')” for redirects, and “return abortNavigation” for cancellation [2]. Example (correct): export default defineNuxtRouteMiddleware((to, from) => { if (to.path!== '/') { return navigateTo('/', { redirectCode: 301 }) } }) 2) In pages/components (e.g., a composable, watch, onMounted, etc.): prefer await (or explicitly return) so you stop further logic - Nuxt’s navigateTo API docs state: “Make sure to always use await or return on result of navigateTo when calling it.” [1] - Practically, in a page you often write “await navigateTo(...)” to ensure execution order (and avoid running code meant for the current page). Nuxt also shows auth/recipe examples that do “await navigateTo('/')” after side effects like refreshing session [3]. Example (typical in pages): await navigateTo('/login') Important nuance - If you call navigateTo and do NOT return/await it (e.g., in middleware), Nuxt warns it can lead to unexpected behavior (because the redirect isn’t properly wired into the middleware flow) [1][2]. - If you redirect on the client side during initial load (client-side middleware), you may see hydration issues; Nuxt community guidance suggests redirecting on the server or after hydration for those cases [4]. So, what should you do for “Nuxt 4 navigateTo return vs await guidance in pages redirect”? - In middleware: return navigateTo(...), not just call it. - In pages (scripts/watches/effects): await navigateTo(...) (or return it from the surrounding async function) and then exit/guard further logic.
Citations:
- 1: https://nuxt.com/docs/4.x/api/utils/navigate-to
- 2: https://nuxt.com/docs/4.x/directory-structure/app/middleware
- 3: https://nuxt.com/docs/4.x/guide/recipes/sessions-and-authentication
- 4: navigateTo causes hydration errors from route middleware (client only) nuxt/nuxt#15136
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- pages/signin.vue (lines 1-80) ----"
if [ -f "pages/signin.vue" ]; then
awk 'NR>=1 && NR<=80 {printf "%5d\t%s\n", NR, $0}' pages/signin.vue | sed -n '1,80p'
else
echo "pages/signin.vue not found"
fi
echo "---- pages/signin.vue (lines 8-20) ----"
if [ -f "pages/signin.vue" ]; then
awk 'NR>=8 && NR<=20 {printf "%5d\t%s\n", NR, $0}' pages/signin.vue
fiRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 1591
Await navigateTo for immediate redirect in <script setup>
In pages/signin.vue (lines 12-14), calling navigateTo('/dashboard') without await/return ignores navigateTo’s returned promise; Nuxt recommends always awaiting/returning navigateTo results.
if (tdeiAuth.complete) {
await navigateTo('/dashboard')
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/signin.vue` around lines 12 - 14, The redirect currently calls
navigateTo('/dashboard') without awaiting its promise; update the branch that
checks tdeiAuth.complete to await (or return) the navigateTo call so Nuxt
handles the navigation immediately—e.g., inside the script setup where
tdeiAuth.complete is checked, replace navigateTo('/dashboard') with await
navigateTo('/dashboard') (or return await navigateTo('/dashboard')) so the
promise is not ignored.
There was a problem hiding this comment.
@shweta2101 Initial observation: if we change the logo here, we'll need to update it in the AVIV ScoutRoute apps as well. Those both display the old Workspaces logo.
Looks nice!
There was a problem hiding this comment.
Thanks for the updates. Visually, the changes look pretty good, and close enough to the TDEI portal. I really appreciate the fix for the mobile navbar.
My main concern is the lack of a coherent design system for these changes. Most of the styles were implemented as brute-force classes rather than extending the existing Bootstrap system, and there are visual patterns that the TDEI portal has that aren't encapsulated for use in other places within the application. For example, the list-card style found on the dashboard is a distinctive visual pattern throughout TDEI, but the styles/components are not set up to reproduce that look and functionality elsewhere.
|
|
||
| :root { | ||
| --primary-font-family: "Open Sans", sans-serif; | ||
| --secondary-font-family: "Montserrat", sans-serif; |
There was a problem hiding this comment.
Do we need to change the font? The original was set up to respect a user's system font settings for better accessibility.
There was a problem hiding this comment.
I added these to align with the TDEI portal's brand typography (Montserrat/Open Sans). However, to respect user system font settings for body text accessibility, I can change the setup to a hybrid approach:
- Use Montserrat only for headings/brand elements (where visual identity is key).
- Revert the body font family (--primary-font-family) to standard system fonts (system-ui, -apple-system, sans-serif)
Let me know if you would like me to update the stylesheets to this hybrid stack!
Introduce a central SCSS theme and migrate components to Bootstrap variants and unified classes. - Add assets/scss/theme.scss with SASS theme variables (brand colors, utility colors, text/nav colors). - Update assets/scss/main.scss to consume the new theme variables, remove many legacy utility classes, and switch several CSS rules to variables/SASS values. - Replace custom tdei button/link classes across components with Bootstrap button variants (btn-primary, btn-outline-secondary, btn-danger, btn-light, etc.). - Refactor AppNavbar to use BootstrapVue collapse/toggle components, improve mobile side-drawer behavior and keyboard focus handling, and normalize class names to kebab-case. - Normalize CSS class names and markup in WorkspaceItem and other components, add theme imports and scoped styles where appropriate, and make small UI/accessibility improvements (icons, alt text, font sizes). - Adjust nuxt.config to limit downloaded font weights and include additional BootstrapVue components used by the navbar. These changes consolidate styling around a single theme, reduce bespoke utility CSS, and align components with Bootstrap conventions for more consistent UI and easier maintenance.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/AppNavbar.vue (1)
120-126: ⚡ Quick winAlign the JS mobile breakpoint with Bootstrap's
md.
isMobileViewflips atinnerWidth <= 768, but the drawer CSS usesmedia-breakpoint-down(md)(max-width: 767.98px) andd-md-nonehides at≥ 768px. At exactly 768px the desktop layout renders whileisMobileViewis stilltrue, so thev-if="mobileMenuOpen && isMobileView"backdrop (Line 93) and an open drawer state can leak onto the desktop view. Use< 768(or read the shared Bootstrapmdvalue) to keep JS and CSS consistent.♻️ Proposed change
- isMobileView.value = window.innerWidth <= 768 + isMobileView.value = window.innerWidth < 768🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/AppNavbar.vue` around lines 120 - 126, The mobile breakpoint logic in syncMobileView() currently sets isMobileView when window.innerWidth <= 768 which mismatches Bootstrap's md breakpoint (max-width: 767.98px) and causes desktop layout bleed at exactly 768px; update syncMobileView() to use window.innerWidth < 768 (or otherwise read the shared Bootstrap md value) and ensure when switching to desktop you still set mobileMenuOpen.value = false so isMobileView and the CSS (d-md-none / media-breakpoint-down(md)) remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/AppNavbar.vue`:
- Around line 18-35: The mobile drawer currently only sets :inert on the
<b-collapse> when closed; implement a proper focus containment by adding an
overlay and toggling inert/aria-hidden/tabindex on the main app content whenever
mobileMenuOpen is true, and add a simple focus trap inside the drawer so
keyboard focus cycles within the <b-collapse> while open; use mobileMenuOpen and
isMobileView to drive this, handle focus management in open/close handlers
(e.g., capture the trigger element when opening, move focus to the first
focusable element inside <b-collapse>, and restore focus to the saved trigger
when side-menu-close button or mobileMenuOpen becomes false). Ensure the
side-menu-close button (and any programmatic close) restores focus and removes
inert/aria-hidden/tabindex on the background.
---
Nitpick comments:
In `@components/AppNavbar.vue`:
- Around line 120-126: The mobile breakpoint logic in syncMobileView() currently
sets isMobileView when window.innerWidth <= 768 which mismatches Bootstrap's md
breakpoint (max-width: 767.98px) and causes desktop layout bleed at exactly
768px; update syncMobileView() to use window.innerWidth < 768 (or otherwise read
the shared Bootstrap md value) and ensure when switching to desktop you still
set mobileMenuOpen.value = false so isMobileView and the CSS (d-md-none /
media-breakpoint-down(md)) remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f651ac4-e83d-4ff5-ac9a-73fe67f20f56
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
assets/scss/main.scssassets/scss/theme.scsscomponents/AppLogo.vuecomponents/AppNavbar.vuecomponents/DatasetTypeRadio.vuecomponents/SigninForm.vuecomponents/dashboard/Toolbar.vuecomponents/dashboard/WorkspaceItem.vuecomponents/review/FilterDropdown.vuecomponents/review/Toolbar.vuecomponents/settings/panel/Delete.vuecomponents/settings/teams/JoinDialog.vuecomponents/teams/Item.vuecomponents/teams/JoinDialog.vuecomponents/teams/MembersDialog.vuecomponents/teams/SettingsDialog.vuenuxt.config.tspages/dashboard.vuepages/help.vuepages/index.vuepages/signin.vuepages/workspace/[id]/export/index.vuepages/workspace/[id]/settings/teams/index.vue
✅ Files skipped from review due to trivial changes (8)
- components/DatasetTypeRadio.vue
- components/settings/panel/Delete.vue
- components/AppLogo.vue
- components/teams/Item.vue
- components/teams/SettingsDialog.vue
- components/teams/MembersDialog.vue
- pages/workspace/[id]/export/index.vue
- components/review/Toolbar.vue
🚧 Files skipped from review as they are similar to previous changes (4)
- components/dashboard/WorkspaceItem.vue
- components/SigninForm.vue
- pages/index.vue
- pages/signin.vue
| <b-collapse | ||
| id="app-nav-collapse" | ||
| v-model="mobileMenuOpen" | ||
| is-nav | ||
| class="app-nav-collapse" | ||
| :inert="!mobileMenuOpen && isMobileView" | ||
| > | ||
| <div class="side-menu-header d-md-none"> | ||
| <app-logo class="logo-sidebar" /> | ||
| <button | ||
| class="side-menu-close" | ||
| type="button" | ||
| aria-label="Close menu" | ||
| @click="mobileMenuOpen = false" | ||
| > | ||
| <app-icon variant="close" size="24" no-margin /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any focus-trap implementation referenced by the navbar/layout.
rg -nP --type=vue -C3 '(focus-?trap|inert|trapFocus|useFocusTrap)' 2>/dev/null
fd -e vue | xargs rg -nP -C3 '(focus-?trap|trapFocus|useFocusTrap)' 2>/dev/nullRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 64
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the navbar file around the cited lines
sed -n '1,200p' components/AppNavbar.vue | nl -ba | sed -n '1,200p'
# 2) Search for focus-trap / focus-containment utilities and directive usage
rg -n --hidden --no-ignore-vcs -S '(focus-?trap|trapFocus|useFocusTrap|inert=|:inert|v-inert)' .
# 3) Search for mobileMenuOpen usage to see if anything else handles focus/aria when open
rg -n --hidden --no-ignore-vcs -S 'mobileMenuOpen' components/AppNavbar.vue components -g'*.vue'Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show AppNavbar.vue (with line numbers) for the relevant area
cat -n components/AppNavbar.vue | sed -n '1,220p'
# 2) Search for focus-trap / focus-containment and inert usage across the repo
rg -n --hidden --no-ignore-vcs -S '(focus-?trap|trapFocus|useFocusTrap|inert=|:inert|v-inert)' .
# 3) Search for mobileMenuOpen wiring across the repo
rg -n --hidden --no-ignore-vcs -S 'mobileMenuOpen' componentsRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 50392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus-trap / focus-containment usage (exclude node_modules)
rg -n --hidden --no-ignore-vcs -S '(focus-?trap|trapFocus|useFocusTrap|focusTrap)' \
--glob '!node_modules/**' .
# Any inert usage in app source (exclude node_modules)
rg -n --hidden --no-ignore-vcs -S '(:inert|v-inert|inert=)' \
--glob '!node_modules/**' .
# Confirm where mobileMenuOpen is referenced (exclude node_modules)
rg -n --hidden --no-ignore-vcs -S 'mobileMenuOpen' \
--glob '!node_modules/**' .Repository: TaskarCenterAtUW/workspaces-frontend
Length of output: 62
🏁 Script executed:
#!/bin/bash
set -euo pipefail
dirs=(components composables directives layouts pages plugins middleware utils)
# List existing dirs (so rg doesn't error)
existing=()
for d in "${dirs[@]}"; do
[ -d "$d" ] && existing+=("$d")
done
echo "Searching in: ${existing[*]:-(none)}"
# Focus-trap / focus-containment terms
rg -n --hidden --no-ignore-vcs -S '(focus-?trap|trapFocus|useFocusTrap|focusTrap)' "${existing[@]:-components}" || true
# inert usage in app source
rg -n --hidden --no-ignore-vcs -S '(:inert|v-inert|inert=)' "${existing[@]:-components}" || true
# mobileMenuOpen references
rg -n --hidden --no-ignore-vcs -S 'mobileMenuOpen' components layouts pages composables plugins || trueRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 1052
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs -S 'mobileMenuOpen.*(aria-hidden|tabindex|focus|inert|hidden)' components layouts pages composables plugins || true
rg -n --hidden --no-ignore-vcs -S '(aria-hidden|tabindex="-1"|role="dialog")' components/AppNavbar.vue components -g'*.vue' || true
rg -n --hidden --no-ignore-vcs -S 'closeOnMobile|side-menu-backdrop|toggleButtonRef' components/AppNavbar.vue || trueRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 1080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Broader focus-trap related terms (source only)
rg -n --hidden --no-ignore-vcs -S '(tabbable|focusin|aria-modal|role="dialog"|focusScope|Dialog|modal.*focus|focus containment|preventScroll|returnFocus|Trap)' \
--glob '!node_modules/**' .
# Any aria-hidden usage tied to mobile menu/drawer/backdrop
rg -n --hidden --no-ignore-vcs -S '(aria-hidden|role="dialog"|tabindex="-1")' \
components/AppNavbar.vue components layouts pages composables plugins || trueRepository: TaskarCenterAtUW/workspaces-frontend
Length of output: 2624
Add focus trapping / background containment for the mobile navbar drawer
- In
components/AppNavbar.vue,:inert="!mobileMenuOpen && isMobileView"only inert(s) the<b-collapse>when the drawer is closed; nothing in the app source marks the background as inert/hidden or applies a focus-trap whilemobileMenuOpenis true. - Add a focus trap (or inert/aria-hidden/tabindex=-1 for the background while open) and restore focus when closing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/AppNavbar.vue` around lines 18 - 35, The mobile drawer currently
only sets :inert on the <b-collapse> when closed; implement a proper focus
containment by adding an overlay and toggling inert/aria-hidden/tabindex on the
main app content whenever mobileMenuOpen is true, and add a simple focus trap
inside the drawer so keyboard focus cycles within the <b-collapse> while open;
use mobileMenuOpen and isMobileView to drive this, handle focus management in
open/close handlers (e.g., capture the trigger element when opening, move focus
to the first focusable element inside <b-collapse>, and restore focus to the
saved trigger when side-menu-close button or mobileMenuOpen becomes false).
Ensure the side-menu-close button (and any programmatic close) restores focus
and removes inert/aria-hidden/tabindex on the background.
Treat 768px as non-mobile by changing the mobile detection from <= 768 to < 768. This aligns the JS breakpoint with expected CSS/UX breakpoints so devices at exactly 768px are handled as desktop and will close the mobile menu.
|
Hi @cyrossignol , Thanks for the feedback. In the initial pass, my focus was on quickly matching the visual layout of the TDEI portal, which led to using custom override classes. I've refactored it: cleaned up the custom classes, set up a central theme.scss extending Bootstrap's variables directly, and scoped the list-card styles inside WorkspaceItem.vue so it is modular and reusable. I also agree that a more comprehensive design system approach would be the ideal long term direction for the application. For this task, my primary goal was to bring the existing React UI experience into the Vue application while keeping the implementation aligned with the current architecture and timeline. As part of addressing the feedback, I've tried to move the implementation closer to reusable patterns without significantly expanding the scope beyond the UI migration itself. Should be much cleaner now. Let me know if that looks better! |
2 participants
DevBoard Task
https://dev.azure.com/TDEI-UW/TDEI/_workitems/edit/3634/
This PR applies TDEI brand theming across the application — updating colours, typography, layout, and accessibility throughout.
Visual changes
#32006e), new responsive mobile side-menu with slide-in animation, user profile dropdown, and sticky positioningtdei-primary-button,tdei-secondary-button,tdei-primary-link,tdei-rounded-button)Screenshots:
Theme & Styling Updates
#9b0092to TDEI brand colour#32006e$brand-accent,$tdei-secondary,$tdei-blue,$tdei-green,$tdei-cyan, background shades, and$text-navymain.scsswith webkit autofill transitions, new CSS variable declarations in:root, and dropdown menu active state styling@nuxtjs/google-fonts) with Open Sans and Montserrat font familiesNavigation & Navbar
AppNavbar.vuefrom Bootstrap collapsed layout to custom responsive design with sticky positioningb-collapsewith click-to-dismiss backdropisMobileViewfor viewport ≤768px) with automatic drawer closure on resizeAuthentication & Sign-in Page
SigninForm.vuewith TDEI-branded auth card layout and updated stylingshowPasswordstateresponse.statusandresponse.json()functionpages/signin.vue) to simplified markup with centered flex layout and login background imageDashboard Layout & Components
WorkspaceItem.vueworkspace picker card with type-specific icons, formatted type labels, conditional status/role pills, and responsive stylingselectedProjectGroupNamecomputed value for project group displayselectWorkspacefunction parameter asWorkspacedashboard/Toolbar.vuebutton styling: "Edit" uses primary variant, "Review/Export/Settings" use outline-secondaryHomepage Updates
hero-shell,hero-copy, andhero-actionsclasses/dashboardwhen authenticated, otherwise to/signin, with dynamic button labelpages/index.vueto importtdeiClientand expose auth state.jumbostyles with new hero-specific classes and responsive breakpoint adjustments across lg/md/sm viewportsComponent Button & Link Standardization
btn btn-outline-primarytobtn btn-outline-secondary(DatasetTypeRadio, help page)btn-darkstyling with appropriate primary/outline-secondary variants in review, settings, and teams componentsAccessibility & UX Improvements
aria-labelin WorkspaceItem to include workspace IDcancelClasswithp-0)Additional Changes
AppLogo.vueto use new TDEI logo (tdei-logo.pnginstead oficon.svg)BCollapseandBNavbarcomponents@nuxtjs/google-fontsto devDependenciesnuxt-loading-indicatorstyling in default layout with gradient color via CSS variables