Skip to content
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

Jump to bottom

aria details vs aria describedby fixed for text field #2352

Merged
merged 2 commits into from
Sep 19, 2024
Merged

aria details vs aria describedby fixed for text field #2352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e57f88
feat: aria details vs aria describedby fixed for text field
tshimber Sep 12, 2024
52e6421
fix: reverted to ariaDetailedId + description added to storybook
tshimber Sep 19, 2024
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
2 changes: 0 additions & 2 deletions packages/components/src/components/text-field/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# scale-text-field



<!-- Auto Generated Below -->


Expand Down
13 changes: 9 additions & 4 deletions packages/components/src/components/text-field/text-field.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,11 @@ export class TextField {
const ariaInvalidAttr =
this.status === 'error' || this.invalid ? { 'aria-invalid': 'true' } : {};
const helperTextId = `helper-message-${this.internalId}`;
const ariaDescribedByAttr = { 'aria-describedBy': helperTextId };
const ariaDetailedById = { 'aria-details': this.ariaDetailedId };
const ariaDescribedByAttr = {
'aria-describedBy':
(this.helperText && helperTextId) || this.ariaDetailedId,
};
const ariaDetailsAttr = { 'aria-details': this.ariaDetailedId };
const numericTypes = [
'number',
'date',
Expand Down Expand Up @@ -275,9 +278,11 @@ export class TextField {
disabled={this.disabled}
readonly={this.readonly}
autocomplete={this.inputAutocomplete}
{...ariaDetailedById}
{...ariaInvalidAttr}
{...(this.helperText ? ariaDescribedByAttr : {})}
{...(this.helperText || this.ariaDetailedId
? ariaDescribedByAttr
: {})}
{...(this.helperText && this.ariaDetailedId ? ariaDetailsAttr : {})}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be a duplicate. One of the ternary operates should be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amir-ba I've done by analogy: each {} block for one property. they both can be set up to conditions
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i mean ternary would be good if it was either ariaDescribedBy or ariaDetails up to conditions. but it can be none, one or both of them

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see , then lets keep it as is

{...(numericTypes.includes(this.type) ? { step: this.step } : {})}
/>
{(!!this.helperText || !!this.counter) && (
Expand Down
45 changes: 45 additions & 0 deletions packages/components/src/html/text-field.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
<meta charset="utf-8" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0"
/>
<title>Stencil Component Starter</title>

<script type="module" src="/build/scale-components.esm.js"></script>
<link rel="stylesheet" href="/build/scale-components.css" />
<style type="text/css" media="screen, print">
html {
margin: 0;
padding: 0;
}

body {
margin: 0;
padding: 4rem;
}
</style>
</head>

<body>
<scale-text-field
label="Label"
placeholder="this is the placeholder"
helper-text="some helper text"
></scale-text-field>
<scale-text-field
label="Label"
placeholder="this is the placeholder"
aria-detailed-id="extra"
></scale-text-field>
<scale-text-field
label="Label"
placeholder="this is the placeholder"
helper-text="some helper text"
aria-detailed-id="extra"
></scale-text-field>
<div id="extra">some extra desc</div>
</body>
</html>
2 changes: 1 addition & 1 deletion packages/storybook-vue/stories/components/text-field/ScaleTextField.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default {
inputAutofocus: { type: Boolean },
inputAutocomplete: { type: String },
experimentalControlled: { type: Boolean },
hideLabelVisually: {type: Boolean, default: false},
hideLabelVisually: { type: Boolean, default: false },
styles: { type: String },
},
methods: {
Expand Down
43 changes: 28 additions & 15 deletions packages/storybook-vue/stories/components/text-field/TextField.stories.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ export const ICON =
description: `(optional) DEPRECATED - css overwrite should replace size`,
control: { type: null },
},
ariaDetailedId: {
table: {
type: { summary: 'string' },
},
description: `(optional) Id of the element with additional information, will be used as aria-details. In case helperText is not provided, will be used as aria-describedBy`,
control: { type: null },
},
}}
/>

Expand Down Expand Up @@ -184,24 +191,30 @@ export const StorySectionsTemplate = (args, { argTypes }) => ({

```css
scale-text-field {
--transition: all var(--telekom-motion-duration-transition)
var(--telekom-motion-easing-standard);
--transition: all var(--telekom-motion-duration-transition) var(
--telekom-motion-easing-standard
);
--radius: var(--telekom-radius-standard);
--border: var(--telekom-spacing-composition-space-01) solid
var(--telekom-color-ui-border-standard);
--border-error: var(--telekom-spacing-composition-space-02) solid
var(--telekom-color-functional-danger-standard);
--border-success: var(--telekom-spacing-composition-space-02) solid
var(--telekom-color-functional-success-standard);
--border-warning: var(--telekom-spacing-composition-space-02) solid
var(--telekom-color-functional-warning-standard);
--border: var(--telekom-spacing-composition-space-01) solid var(
--telekom-color-ui-border-standard
);
--border-error: var(--telekom-spacing-composition-space-02) solid var(
--telekom-color-functional-danger-standard
);
--border-success: var(--telekom-spacing-composition-space-02) solid var(
--telekom-color-functional-success-standard
);
--border-warning: var(--telekom-spacing-composition-space-02) solid var(
--telekom-color-functional-warning-standard
);
--border-color-hover: var(--telekom-color-ui-border-hovered);
--border-color-focus: var(--telekom-color-ui-border-hovered);
--border-color-disabled: var(--telekom-color-ui-border-disabled);
--background-color-hover: var(--telekom-color-ui-state-fill-hovered);
--background-color-disabled: none;
--focus-outline: var(--telekom-line-weight-highlight) solid
var(--telekom-color-functional-focus-standard);
--focus-outline: var(--telekom-line-weight-highlight) solid var(
--telekom-color-functional-focus-standard
);
--height: 44px;
--spacing-x: var(--telekom-spacing-composition-space-05);
--color-disabled: var(--telekom-color-text-and-icon-disabled);
Expand All @@ -218,9 +231,9 @@ scale-text-field {
--color-meta-error: var(--telekom-color-text-and-icon-functional-danger);

/* control */
--spacing-control: 1.125rem
calc(2rem - var(--telekom-spacing-composition-space-01)) 0.25rem
calc(0.75rem - var(--telekom-spacing-composition-space-01));
--spacing-control: 1.125rem calc(
2rem - var(--telekom-spacing-composition-space-01)
) 0.25rem calc(0.75rem - var(--telekom-spacing-composition-space-01));
--transition-control: var(--transition);
--background-control: var(--telekom-color-ui-state-fill-standard);
--margin-bottom-control: var(--telekom-spacing-composition-space-03);
Expand Down
Loading