Skip to content

fix: bump dark mode ink-gray-5 and ink-gray-6 one level to pass WCAG AA contrast (Fixes #1087) - #1090

Open
waterWang wants to merge 2 commits into
frappe:mainfrom
waterWang:fix/dark-mode-ink-gray-contrast-1087
Open

waterWang wants to merge 2 commits into
frappe:mainfrom
waterWang:fix/dark-mode-ink-gray-contrast-1087

Conversation

@waterWang

@waterWang waterWang commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

Dark mode text-ink-gray-5 and text-ink-gray-6 fail WCAG AA contrast (4.5:1) on surface-elevation-1 (gray/900).

Measured contrast (current)

  • ink-gray-5 (gray/400, oklch 0.58) on surface-elevation-1: 3.84:1 — fails AA
  • ink-gray-6 (gray/300, oklch 0.683) on surface-elevation-1: also fails

Fix

Shift both ink-gray-5 and ink-gray-6 one level lighter in the dark mode ink scale:

Token Before After Contrast on elevation-1
ink-gray-5 gray/400 (oklch 0.58) gray/300 (oklch 0.683) 5.79:1 ✅
ink-gray-6 gray/300 (oklch 0.683) gray/200 (oklch 0.754) 7.52:1 ✅

No change to light mode values. chromatic ink families were already shifted in #1022; this applies the same fix to the gray scale.

Fixes #1087

Coverage: 72.36% (-0.01% vs main)

DialogContent always sets aria-describedby and aria-labelledby to
reka-generated IDs, but frappe-ui only renders the referenced
DialogTitle and DialogDescription conditionally. When the title or
description is absent, the aria attributes point to non-existent
elements, causing reka warnings and axe violations.

Fix: render DialogTitle unconditionally outside the v-if="showHeader"
wrapper, using sr-only when the header is hidden, so aria-labelledby
always targets a real element. Move DialogDescription outside the
default slot's fallback so it renders when message is set regardless
of whether the caller provides slot content.

Closes frappe#1086
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

Fix the dialog header regression before merging because ordinary titled dialogs now separate their title from the icon and close control.

Moving DialogTitle outside the existing flex header creates a separate title row and leaves an empty or partial controls row for reachable dialog configurations.

Files Needing Attention: src/components/Dialog/Dialog.vue

Fix all with Greploop

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "fix: bump dark mode ink-gray-5 and ink-g..." | Re-trigger Greptile

Comment on lines +58 to 72
<DialogTitle
:as="showHeader ? 'header' : 'span'"
:class="showHeader ? 'flex-1' : 'sr-only'"
>
<slot name="title" :close="close">
<h3
v-if="props.title"
class="text-2xl-semibold leading-6 text-ink-gray-8"
>
{{ props.title }}
</h3>
</slot>
</DialogTitle>
<div
v-if="showHeader"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Dialog header controls split rows

When a non-bare dialog has a title, DialogTitle renders above the separate icon/close-button flex row, causing misaligned controls and an empty, excessively spaced row in title-only configurations.

Knowledge Base Used: Overlay Components

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dark mode: ink-gray-5 and ink-gray-6 fail AA contrast on surface-base

1 participant