Skip to content

feat(a11y): add reusable Alert component #1955

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.

Sign up for GitHub

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

Merged
shuuji3 merged 8 commits into from
Mar 11, 2026
+106 −42
Merged

feat(a11y): add reusable Alert component #1955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b406716
feat: build a common alert component
ShroXd Mar 6, 2026
f7ebde3
refactor: use alert component
ShroXd Mar 6, 2026
470daae
feat: tweaks the color pattern of alert component
ShroXd Mar 6, 2026
cad166f
test: add a11y test for alert component
ShroXd Mar 6, 2026
06db88f
fix: adjust role in Alert component
ShroXd Mar 6, 2026
d89b38b
test: cover theme-specific contrast in a11y test
ShroXd Mar 6, 2026
ae315f0
Merge branch 'main' into create-alert-component
ShroXd Mar 9, 2026
23b187b
refactor: use screaming snake case for constants
ghostdevv Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions app/components/Alert.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<script setup lang="ts">
interface Props {
variant: 'warning' | 'error'
title?: string
}

defineProps<Props>()

const WRAPPER_CLASSES: Record<Props['variant'], string> = {
warning: 'border-amber-400/20 bg-amber-500/8',
error: 'border-red-400/20 bg-red-500/8',
}

const TITLE_CLASSES: Record<Props['variant'], string> = {
warning: 'text-amber-800 dark:text-amber-300',
error: 'text-red-800 dark:text-red-300',
}

const BODY_CLASSES: Record<Props['variant'], string> = {
warning: 'text-amber-700 dark:text-amber-400',
error: 'text-red-700 dark:text-red-400',
}

const ROLES: Record<Props['variant'], 'status' | 'alert'> = {
warning: 'status',
error: 'alert',
}
</script>

<template>
<div
:role="ROLES[variant]"
class="border rounded-md px-3 py-2.5"
:class="WRAPPER_CLASSES[variant]"
>
<p v-if="title" class="font-semibold mb-1" :class="TITLE_CLASSES[variant]">{{ title }}</p>
<div class="text-xs" :class="BODY_CLASSES[variant]">
<slot />
</div>
</div>
</template>
61 changes: 19 additions & 42 deletions app/components/Package/ClaimPackageModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,28 +199,26 @@ const previewPackageJson = computed(() => {
</div>

<!-- Validation errors -->
<div
<Alert
v-if="checkResult.validationErrors?.length"
class="p-3 text-sm text-red-400 bg-red-500/10 border border-red-500/20 rounded-md"
role="alert"
variant="error"
:title="$t('claim.modal.invalid_name')"
>
<p class="font-medium mb-1">{{ $t('claim.modal.invalid_name') }}</p>
<ul class="list-disc list-inside space-y-1">
<li v-for="err in checkResult.validationErrors" :key="err">{{ err }}</li>
</ul>
</div>
</Alert>

<!-- Validation warnings -->
<div
<Alert
v-if="checkResult.validationWarnings?.length"
class="p-3 text-sm text-yellow-400 bg-yellow-500/10 border border-yellow-500/20 rounded-md"
role="alert"
variant="warning"
:title="$t('common.warnings')"
>
<p class="font-medium mb-1">{{ $t('common.warnings') }}</p>
<ul class="list-disc list-inside space-y-1">
<li v-for="warn in checkResult.validationWarnings" :key="warn">{{ warn }}</li>
</ul>
</div>
</Alert>

<!-- Availability status -->
<template v-if="checkResult.valid">
Expand Down Expand Up @@ -305,39 +303,23 @@ const previewPackageJson = computed(() => {
</template>

<!-- Error message -->
<div
v-if="mergedError"
role="alert"
class="p-3 text-sm text-red-400 bg-red-500/10 border border-red-500/20 rounded-md"
>
{{ mergedError }}
</div>
<Alert v-if="mergedError" variant="error">{{ mergedError }}</Alert>
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This publish-error alert never renders.

mergedError is computed as null whenever checkResult exists, so once the availability check has succeeded this branch cannot show failures from handleClaim(). If publishing fails, the modal stays open without surfacing the error. Please separate availability-check errors from publish errors, or let publishError win before the checkResult !== null guard.

One way to untangle the two error paths 
-const mergedError = computed(() => {
-  return checkResult.value !== null
-    ? null
-    : (publishError.value ??
-        (checkError.value instanceof Error
-          ? checkError.value.message
-          : $t('claim.modal.failed_to_check')))
-})
+const availabilityError = computed(() => {
+  if (checkResult.value !== null) return null
+  return checkError.value instanceof Error
+    ? checkError.value.message
+    : $t('claim.modal.failed_to_check')
+})
+
+const mergedError = computed(() => {
+  return publishError.value ?? availabilityError.value
+})

Copy link
Contributor Author

@ShroXd ShroXd Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is existing logic, since this PR only focuses on the UI part, ignore it for now.

Copy link
Contributor

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShroXd, understood — that logic predates this PR and is outside its scope. I'll leave it for a future fix. The UI refactoring with the new Alert component looks clean and the before/after screenshots show a clear improvement in contrast for both light and dark themes.

🧠 Learnings used 
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 838
File: app/pages/package/[...package].vue:445-449
Timestamp: 2026-02-03T13:59:33.392Z
Learning: The copy button pattern in app/pages/package/[...package].vue may be made into a reusable component or pattern in the future, but currently it's acceptable to keep it inline with the CSS-only approach for smooth animations.

Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/AccessControls.vue:253-253
Timestamp: 2026-02-04T05:34:20.527Z
Learning: In the npmx.dev project, ensure that focus-visible styling for button and select elements is implemented globally in app/assets/main.css with the rule: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements in Vue templates or components (e.g., AccessControls.vue). Rely on the global rule for consistency and maintainability; only use inline focus-visible utilities when styling non-button/select elements or in exceptional cases outside the global scope.

Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/Versions.vue:332-332
Timestamp: 2026-02-04T05:34:54.335Z
Learning: In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via main.css with: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Therefore, individual buttons or selects in Vue components should not rely on inline focus-visible utility classes like focus-visible:outline-accent/70. Ensure components do not add per-element focus-visible utilities; rely on the global rule. If a component seems to require a different focus state, adjust only through global CSS or maintain accessibility with native focus styles, rather than inserting inline utility classes.

Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1170
File: app/components/Package/MetricsBadges.vue:66-66
Timestamp: 2026-02-08T15:02:02.232Z
Learning: In Vue components that use UnoCSS with the preset-icons collection, prefer colon-syntax for icons (e.g., i-carbon:checkmark) over the dash-separated form (i-carbon-checkmark). This aids UnoCSS in resolving the correct collection directly, which improves performance for long icon names. Apply this pattern to all Vue components (e.g., app/components/**/*.vue) where UnoCSS icons are used; ensure UnoCSS is configured with the preset-icons collection.

Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1335
File: app/components/Compare/FacetSelector.vue:72-78
Timestamp: 2026-02-10T15:47:33.467Z
Learning: In the npmx.dev project, ButtonBase (used via app/components/ButtonBase.vue or similar) provides default classes: border border-border. When styling ButtonBase instances in Vue components (e.g., app/components/Compare/FacetSelector.vue and other files under app/components), avoid duplicating the border class to prevent the HTML validator error no-dup-class and CI failures. If styling overrides are needed, ensure only unique classes are applied (remove redundant border classes or adjust via props) so the default border remains intact without duplication.

Learnt from: abbeyperini
Repo: npmx-dev/npmx.dev PR: 1049
File: app/components/Settings/Toggle.client.vue:22-29
Timestamp: 2026-02-11T00:01:33.121Z
Learning: In Vue 3.4 and later, you can use same-name shorthand for attribute bindings: use :attributeName instead of :attributeName="attributeName" when binding to a variable with the same name in scope. For example, :id is equivalent to :id="id" when an id variable exists. Apply this shorthand in .vue components (notably in Settings/Toggle.client.vue) to simplify templates. Ensure the bound variable exists and that you are using a Vue version that supports this shorthand.

Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1845
File: app/components/InstantSearch.vue:6-11
Timestamp: 2026-03-03T09:42:52.533Z
Learning: Maintain the established prehydration pattern across the project: use JSON.parse(localStorage.getItem('npmx-settings') || '{}') without per-call try-catch blocks. Do not introduce try-catch error handling for this pattern unless a coordinated, project-wide refactor of all onPrehydrate readers is planned and executed.


<!-- Actions -->
<div v-if="checkResult.available && checkResult.valid" class="space-y-3">
<!-- Warning for unscoped packages -->
<div
v-if="!isScoped"
class="p-3 text-sm text-yellow-400 bg-yellow-500/10 border border-yellow-500/20 rounded-md"
>
<p class="font-medium mb-1">{{ $t('claim.modal.scope_warning_title') }}</p>
<p class="text-xs text-yellow-400/80">
{{
$t('claim.modal.scope_warning_text', {
username: npmUser || 'username',
name: packageName,
})
}}
</p>
</div>
<Alert v-if="!isScoped" variant="warning" :title="$t('claim.modal.scope_warning_title')">
{{
$t('claim.modal.scope_warning_text', {
username: npmUser || 'username',
name: packageName,
})
}}
</Alert>

<!-- Not connected warning -->
<div v-if="!isConnected" class="space-y-3">
<div
class="p-3 text-sm text-yellow-400 bg-yellow-500/10 border border-yellow-500/20 rounded-md"
>
<p>{{ $t('claim.modal.connect_required') }}</p>
</div>
<Alert variant="warning">{{ $t('claim.modal.connect_required') }}</Alert>
<button
type="button"
class="w-full px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md transition-colors duration-200 hover:bg-fg/90 focus-visible:outline-accent/70"
Expand Down Expand Up @@ -389,12 +371,7 @@ const previewPackageJson = computed(() => {

<!-- Error state -->
<div v-else-if="mergedError" class="space-y-4">
<div
role="alert"
class="p-3 text-sm text-red-400 bg-red-500/10 border border-red-500/20 rounded-md"
>
{{ mergedError }}
</div>
<Alert variant="error">{{ mergedError }}</Alert>
<button
type="button"
class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-accent/70"
Expand Down
46 changes: 46 additions & 0 deletions test/nuxt/a11y.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ vi.mock('vue-data-ui/vue-ui-xy', () => {
// Import components from #components where possible
// For server/client variants, we need to import directly to test the specific variant
import {
Alert,
AppFooter,
AppHeader,
AppLogo,
Expand Down Expand Up @@ -3529,6 +3530,35 @@ describe('component accessibility audits', () => {
expect(results.violations).toEqual([])
})
})

describe('Alert', () => {
it('should have no accessibility violations for warning variant', async () => {
const component = await mountSuspended(Alert, {
props: { variant: 'warning', title: 'Warning title' },
slots: { default: 'This is a warning message.' },
})
const results = await runAxe(component)
expect(results.violations).toEqual([])
})

it('should have no accessibility violations for error variant', async () => {
const component = await mountSuspended(Alert, {
props: { variant: 'error', title: 'Error title' },
slots: { default: 'This is an error message.' },
})
const results = await runAxe(component)
expect(results.violations).toEqual([])
})

it('should have no accessibility violations without title', async () => {
const component = await mountSuspended(Alert, {
props: { variant: 'warning' },
slots: { default: 'This is a warning message.' },
})
const results = await runAxe(component)
expect(results.violations).toEqual([])
})
})
})

function applyTheme(colorMode: string, bgTheme: string | null) {
Expand Down Expand Up @@ -3575,6 +3605,22 @@ describe('background theme accessibility', () => {
}

const components = [
{
name: 'AlertWarning',
mount: () =>
mountSuspended(Alert, {
props: { variant: 'warning', title: 'Warning title' },
slots: { default: '<p>Warning body</p>' },
}),
},
{
name: 'AlertError',
mount: () =>
mountSuspended(Alert, {
props: { variant: 'error', title: 'Error title' },
slots: { default: '<p>Error body</p>' },
}),
},
{ name: 'AppHeader', mount: () => mountSuspended(AppHeader) },
{ name: 'AppFooter', mount: () => mountSuspended(AppFooter) },
{ name: 'HeaderSearchBox', mount: () => mountSuspended(HeaderSearchBox) },
Expand Down
Loading