Skip to content

Commit 8f6b741

Browse files
authored
feat: remove excessive description from "Add a secret" dialog (#491)
* feat: remove excessive description from "Add a secret" dialog note: the description is an accessibility requirement for screen readers and our tests will fail without it, because radix ui issues console warnings for it, and we fail our tests on console warnings. this behavior makes sense, so instead of disabling it, I decided to keep the description for screen readers (hence the sr-only class). In order to make the behavior less confusing when refactoring/debugging this area of the app, I also added a test case that verifies the screen-reader behavior. * replicate the same behavior for editing secrets * refactor * reduce distance between header and form in "add a secret"
1 parent 9956df8 commit 8f6b741

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

renderer/src/features/secrets/components/dialog-form-secret.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export function DialogFormSecret({
8686
>
8787
<DialogHeader>
8888
<DialogTitle>{title}</DialogTitle>
89-
<DialogDescription>
89+
<DialogDescription className="sr-only">
9090
{secretKey
9191
? 'Update the secret value below.'
9292
: 'Enter a name and value for your new secret.'}
@@ -116,7 +116,7 @@ function SecretForm({ form, isEditMode, onSubmit, onCancel }: SecretFormProps) {
116116
return (
117117
<Form {...form}>
118118
<form onSubmit={form.handleSubmit(handleFormSubmit)}>
119-
<div className="space-y-4 py-8">
119+
<div className="space-y-4 pt-4 pb-8">
120120
{!isEditMode && (
121121
<FormField
122122
control={form.control}

renderer/src/routes/__tests__/secrets.test.tsx

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,22 @@ it('renders add secret dialog when clicking add secret button', async () => {
5050
const addSecretButton = screen.getByRole('button', { name: /add secret/i })
5151
await userEvent.click(addSecretButton)
5252

53-
expect(
54-
screen.getByText('Enter a name and value for your new secret.')
55-
).toBeInTheDocument()
5653
expect(screen.getByPlaceholderText('Name')).toBeInTheDocument()
5754
expect(screen.getByPlaceholderText('Secret')).toBeInTheDocument()
5855
expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument()
5956
expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument()
57+
58+
// Accessibility: check sr-only description and aria-describedby
59+
const dialog = screen.getByRole('dialog')
60+
const srDescription = Array.from(dialog.querySelectorAll('p,div,span')).find(
61+
(el) =>
62+
el.className.includes('sr-only') &&
63+
el.textContent?.includes('Enter a name and value for your new secret.')
64+
)
65+
expect(srDescription).toBeTruthy()
66+
const ariaDescribedBy = dialog.getAttribute('aria-describedby')
67+
expect(ariaDescribedBy).toBeTruthy()
68+
expect(srDescription?.id).toBe(ariaDescribedBy)
6069
})
6170

6271
it('renders edit secret dialog when clicking edit from dropdown', async () => {
@@ -78,4 +87,16 @@ it('renders edit secret dialog when clicking edit from dropdown', async () => {
7887
expect(screen.getByPlaceholderText('Secret')).toBeInTheDocument()
7988
expect(screen.getByRole('button', { name: /update/i })).toBeInTheDocument()
8089
expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument()
90+
91+
// Accessibility: check sr-only and aria-describedby
92+
const dialog = screen.getByRole('dialog')
93+
const srDescription = Array.from(dialog.querySelectorAll('p,div,span')).find(
94+
(el) =>
95+
el.className.includes('sr-only') &&
96+
el.textContent?.includes('Update the secret value below.')
97+
)
98+
expect(srDescription).toBeTruthy()
99+
const ariaDescribedBy = dialog.getAttribute('aria-describedby')
100+
expect(ariaDescribedBy).toBeTruthy()
101+
expect(srDescription?.id).toBe(ariaDescribedBy)
81102
})

0 commit comments

Comments
 (0)