Skip to content

Commit

Permalink
Make asterisk a pseudo element to avoid confusing test requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
NicholasBoll committed May 17, 2024
1 parent 063da12 commit cb2a3a3
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
23 changes: 22 additions & 1 deletion modules/docs/mdx/11.0-UPGRADE-GUIDE.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,8 @@ variant prop.

### Form Field (Preview)

**PR:** [#2472](https://github.com/Workday/canvas-kit/pull/2472)
**PR:** [#2472](https://github.com/Workday/canvas-kit/pull/2472),
[#2746](https://github.com/Workday/canvas-kit/pull/2746)

`FormField` in [Preview](#preview) is a compound component and we intend to promote it in a future
version to replace the `FormField` in [Main](#main). Because of this, we've refactored how errors
Expand All @@ -538,6 +539,26 @@ are applied to `FormField` in [Preview](#preview) in order to match the API from
- `FormField` does **not** support the `useFieldSet` prop that the `FormField` in [Main](#main)
does. In order to achieve the same behavior, set the `as` prop on the `FormField` element to
`fieldset` and the `as` prop of `FormField.Label` to `legend`.
- The required asterisk is now a pseudo element. While the asterisk was never read out loud by
screen readers, Testing Library required it in the `*ByLabelText` query. `*ByRole` uses the w3c
[accessible name calculation specification](https://www.w3.org/TR/accname-1.2/), but
`*ByLabelText` uses
[textContent](https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent). Additional
information: https://github.com/testing-library/dom-testing-library/issues/929

v10:

```ts
screen.getByLabelText('Email*'); // Email is a required field and asterisk is included in name
screen.getByRole('textbox', {name: 'Email'}); // correctly ignores the `*`
```

v11:

```ts
screen.getByLabelText('Email');
screen.getByRole('textbox', {name: 'Email'});
```

```tsx
// v10 FormField in Preview
Expand Down
15 changes: 11 additions & 4 deletions modules/preview-react/form-field/lib/FormFieldLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,16 @@ export const formFieldLabelStencil = createStencil({
marginInlineStart: system.space.x1,
},

// asterisk element
// asterisk
[parentModifier(formFieldStencil.modifiers.required.true)]: {
display: 'inline',
'&::after': {
content: '"*"',
fontSize: system.fontSize.body.large,
fontWeight: system.fontWeight.normal,
color: brand.error.base,
textDecoration: 'unset',
marginInlineStart: system.space.x1,
},
},

// orientation modifier from parent FormField
Expand All @@ -66,9 +73,9 @@ export const FormFieldLabel = createSubcomponent('label')({
return (
<Element {...mergeStyles(elemProps, formFieldLabelStencil({typeLevel, variant}))}>
{children}
<span data-element="asterisk" aria-hidden="true">
{/* <span data-element="asterisk" aria-hidden="true">
*
</span>
</span> */}
</Element>
);
});
6 changes: 1 addition & 5 deletions modules/preview-react/form-field/lib/formFieldStencil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ export const formFieldStencil = createStencil({
},
},
required: {
true: {
'& [data-element=asterisk]': {
display: 'block',
},
},
true: {},
},
error: {
error: {},
Expand Down

0 comments on commit cb2a3a3

Please sign in to comment.