Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(OnyxErrorTooltip): use onyx tooltip for errors on checkbox/radio/switch #1694

Merged
merged 48 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
692fab6
add tooltip
BoppLi Jul 25, 2024
cffff82
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 5, 2024
38e3b3d
feat(OnyxErrorTooltip): create component
BoppLi Aug 5, 2024
cbc6ce2
use onyxErrorTooltip for checkbox, radio and switch
BoppLi Aug 5, 2024
ded5cf8
remove title from customValidity again
BoppLi Aug 5, 2024
0434314
add spacing for error tooltips
BoppLi Aug 5, 2024
3a4920c
add comment
BoppLi Aug 5, 2024
2b03f83
fix(OnyxRadioButton): hide error when one radio button is checked in …
BoppLi Aug 5, 2024
d55e946
feat(OnyxErrorTooltip): render with or without tooltip depending on e…
BoppLi Aug 5, 2024
2aae41d
simplify targetRef logic
BoppLi Aug 5, 2024
53dee40
add documentation
BoppLi Aug 5, 2024
96d410e
cleanup
BoppLi Aug 5, 2024
febb938
remove icon from error tooltip, adjust layout of storybook examples
BoppLi Aug 5, 2024
152e05c
hide error tooltip before user interaction
BoppLi Aug 5, 2024
d87fe06
fix visibility condition
BoppLi Aug 5, 2024
300d4c9
adjust tests for checkbox
BoppLi Aug 5, 2024
19f2dba
implement tests for radio button
BoppLi Aug 5, 2024
1ae22d5
write tests for switch
BoppLi Aug 5, 2024
2a84cf5
cleanup
BoppLi Aug 5, 2024
12858a9
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 5, 2024
9ff0d9f
update screenshots
BoppLi Aug 6, 2024
6d0f3fe
use boolean control for onyxSelect
BoppLi Aug 6, 2024
9ea1ed0
move style to decorators to prevent console warnings
BoppLi Aug 6, 2024
0a59846
prevent storybook bugs due to multiple template roots of OnyxErrorToo…
BoppLi Aug 6, 2024
492e66f
clean up cleared todos
BoppLi Aug 6, 2024
03fca70
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 6, 2024
8b5e344
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 6, 2024
c34cd84
reset screenshots
BoppLi Aug 6, 2024
1c82c46
update screenshots
BoppLi Aug 6, 2024
809663a
stabilize screenshot test
BoppLi Aug 6, 2024
7f9e6ff
add component tests
BoppLi Aug 6, 2024
500c282
stabilize tests
BoppLi Aug 6, 2024
dadae81
fix error
BoppLi Aug 6, 2024
328f7a5
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 6, 2024
990553b
stabilize test
BoppLi Aug 6, 2024
dd0b4d2
modify tests
BoppLi Aug 6, 2024
6e08b6a
fix checkbox tests
BoppLi Aug 7, 2024
6a49a72
update screenshots
BoppLi Aug 7, 2024
0f278ba
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 7, 2024
661cb74
Merge branch 'feat/1600-use-onyx-tooltip-for-error-and-hidden-labels'…
BoppLi Aug 7, 2024
274ea2a
stabilize switch test
BoppLi Aug 7, 2024
b75768d
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 7, 2024
584c466
update tests
BoppLi Aug 7, 2024
d9ab001
remove unused file
BoppLi Aug 7, 2024
200bfa8
add layers mixin
BoppLi Aug 7, 2024
a76d0eb
Merge branch 'main' into feat/1600-use-onyx-tooltip-for-error-and-hid…
BoppLi Aug 7, 2024
5acb268
enhance radio group invalid screenshot
BoppLi Aug 7, 2024
9577985
chore: update Playwright screenshots (#1724)
github-actions[bot] Aug 8, 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
93 changes: 53 additions & 40 deletions packages/sit-onyx/src/components/OnyxCheckbox/OnyxCheckbox.ct.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,38 @@ test.describe("Screenshot tests", () => {

executeMatrixScreenshotTest({
name: "Checkbox (invalid)",
columns: ["unchecked", "indeterminate", "checked", "disabled", "hideLabel"],
columns: ["unchecked", "indeterminate", "checked", "hideLabel", "longError", "disabled"],
rows: ["default", "hover", "focus-visible"],
component: (column) => (
<OnyxCheckbox
label="Test label"
modelValue={column === "checked"}
indeterminate={column === "indeterminate"}
hideLabel={column === "hideLabel"}
disabled={column === "disabled"}
customError="Test error"
value="test-value"
/>
),
component: (column, row) => {
const customError =
column === "longError"
? { shortMessage: "Error", longMessage: "Further info" }
: "Test error";
return (
<OnyxCheckbox
style={row !== "default" ? "padding-top: 3rem;" : ""}
label="Test label"
modelValue={column === "checked"}
indeterminate={column === "indeterminate"}
hideLabel={column === "hideLabel"}
disabled={column === "disabled"}
customError={customError}
value="test-value"
/>
);
},
beforeScreenshot: async (component, page, column, row) => {
const checkbox = component.getByLabel("Test label");

if (column !== "disabled") {
// invalid only shows if checkbox is touched
await checkbox.focus();
await page.keyboard.press("Space");
await page.keyboard.press("Space");

await checkbox.click();
await checkbox.click();
await page.getByRole("document").click(); // reset focus
if (row !== "focus-visible") {
await checkbox.blur(); // reset focus
}

if (column === "indeterminate") {
await checkbox.evaluate(
Expand All @@ -99,8 +109,18 @@ test.describe("Screenshot tests", () => {
}
}

if (row === "hover") await component.hover();
if (row === "focus-visible") await page.keyboard.press("Tab");
if (row === "hover" && column !== "disabled") {
await checkbox.hover();
}

// wait for the tooltip to show up reliably
if (["focus-visible", "hover"].includes(row) && column !== "disabled") {
// eslint-disable-next-line playwright/no-standalone-expect
await expect(
component.getByRole("tooltip"),
`should show error tooltip for ${row} and ${column}`,
).toBeVisible();
Dismissed Show dismissed Hide dismissed
}
},
});

Expand Down Expand Up @@ -158,27 +178,20 @@ test.describe("Screenshot tests", () => {
});
});

[
{ hideLabel: true, customError: undefined, expectedTitle: "Label" },
{ hideLabel: true, customError: "Error", expectedTitle: "Label\nError" },
{ hideLabel: false, customError: "Error", expectedTitle: "Error" },
{
hideLabel: false,
customError: { shortMessage: "Error", longMessage: "Further info" },
expectedTitle: "Error: Further info",
},
].forEach(({ hideLabel, customError, expectedTitle }) => {
test(`should have the title "${expectedTitle}"`, async ({ mount, makeAxeBuilder, page }) => {
// ARRANGE
await mount(<OnyxCheckbox label="Label" hideLabel={hideLabel} customError={customError} />);

// ASSERT
await expect(page.getByTitle(expectedTitle), "should have the expected title").toBeVisible();

// ACT
const accessibilityScanResults = await makeAxeBuilder().analyze();

// ASSERT
expect(accessibilityScanResults.violations).toEqual([]);
});
test("should have the title show the label as title if hideLabel is set", async ({
mount,
makeAxeBuilder,
page,
}) => {
// ARRANGE
await mount(<OnyxCheckbox label="Demo Label" hideLabel value="test-value" />);

// ASSERT
await expect(page.getByTitle("Demo Label"), "should have the expected title").toBeVisible();

// ACT
const accessibilityScanResults = await makeAxeBuilder().analyze();

// ASSERT
expect(accessibilityScanResults.violations).toEqual([]);
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ export const Required = {
...Default.args,
required: true,
},
decorators: [
BoppLi marked this conversation as resolved.
Show resolved Hide resolved
(story) => ({
components: { story },
template: `
<div style="padding: 2rem 0 0 2.5rem;">
<story />
</div>`,
}),
],
} satisfies Story;

/**
Expand All @@ -64,6 +73,7 @@ export const Required = {
export const HiddenLabel = {
args: {
...Default.args,
label: "Hidden label",
hideLabel: true,
},
} satisfies Story;
Expand Down Expand Up @@ -98,8 +108,17 @@ export const CustomError = {
...Default.args,
hideLabel: true,
customError: {
shortMessage: "Example custom error",
longMessage: "This text might inform the users what they can do to fix the error.",
shortMessage: "Custom error",
longMessage: "Further explanation.",
},
},
decorators: [
(story) => ({
components: { story },
template: `
<div style="padding: 2rem 0 0 5rem;">
<story />
</div>`,
}),
],
} satisfies Story;
88 changes: 50 additions & 38 deletions packages/sit-onyx/src/components/OnyxCheckbox/OnyxCheckbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useDensity } from "../../composables/density";
import { useRequired } from "../../composables/required";
import { useCustomValidity } from "../../composables/useCustomValidity";
import type { SelectOptionValue } from "../../types";
import OnyxErrorTooltip from "../OnyxErrorTooltip/OnyxErrorTooltip.vue";
import OnyxLoadingIndicator from "../OnyxLoadingIndicator/OnyxLoadingIndicator.vue";
import OnyxSkeleton from "../OnyxSkeleton/OnyxSkeleton.vue";
import type { OnyxCheckboxProps } from "./types";
Expand Down Expand Up @@ -35,7 +36,11 @@ const isChecked = computed({
const { requiredMarkerClass, requiredTypeClass } = useRequired(props);
const { densityClass } = useDensity(props);

const { vCustomValidity, title } = useCustomValidity({ props, emit });
const { vCustomValidity, errorMessages } = useCustomValidity({ props, emit });

const title = computed(() => {
return props.hideLabel ? props.label : undefined;
});
</script>

<template>
Expand All @@ -44,43 +49,45 @@ const { vCustomValidity, title } = useCustomValidity({ props, emit });
<OnyxSkeleton v-if="!props.hideLabel" class="onyx-checkbox-skeleton__label" />
</div>

<label v-else class="onyx-checkbox" :class="[requiredTypeClass, densityClass]" :title="title">
<div class="onyx-checkbox__container">
<OnyxLoadingIndicator v-if="props.loading" class="onyx-checkbox__loading" type="circle" />
<input
v-else
v-model="isChecked"
v-custom-validity
:aria-label="props.hideLabel ? props.label : undefined"
class="onyx-checkbox__input"
type="checkbox"
:indeterminate="props.indeterminate"
:disabled="props.disabled"
:required="props.required"
:value="props.value"
:autofocus="props.autofocus"
/>
</div>

<template v-if="!props.hideLabel">
<p
class="onyx-checkbox__label"
:class="[
`onyx-truncation-${props.truncation}`,
// shows the required marker inline for multiline labels
props.truncation === 'multiline' ? requiredMarkerClass : undefined,
]"
>
{{ props.label }}
</p>
<!-- shows the required marker fixed on the right for truncated labels -->
<div
v-if="props.truncation === 'ellipsis'"
class="onyx-checkbox__marker"
:class="[requiredMarkerClass]"
></div>
</template>
</label>
<OnyxErrorTooltip v-else :disabled="props.disabled" :error-messages="errorMessages">
<label class="onyx-checkbox" :class="[requiredTypeClass, densityClass]" :title="title">
<div class="onyx-checkbox__container">
<OnyxLoadingIndicator v-if="props.loading" class="onyx-checkbox__loading" type="circle" />
<input
v-else
v-model="isChecked"
v-custom-validity
:aria-label="props.hideLabel ? props.label : undefined"
class="onyx-checkbox__input"
type="checkbox"
:indeterminate="props.indeterminate"
:disabled="props.disabled"
:required="props.required"
:value="props.value"
:autofocus="props.autofocus"
/>
</div>

<template v-if="!props.hideLabel">
<p
class="onyx-checkbox__label"
:class="[
`onyx-truncation-${props.truncation}`,
// shows the required marker inline for multiline labels
props.truncation === 'multiline' ? requiredMarkerClass : undefined,
]"
>
{{ props.label }}
</p>
<!-- shows the required marker fixed on the right for truncated labels -->
<div
v-if="props.truncation === 'ellipsis'"
class="onyx-checkbox__marker"
:class="[requiredMarkerClass]"
></div>
</template>
</label>
</OnyxErrorTooltip>
</template>

<style lang="scss">
Expand Down Expand Up @@ -164,6 +171,11 @@ const { vCustomValidity, title } = useCustomValidity({ props, emit });
max-width: var(--onyx-checkbox-input-size);
height: var(--onyx-checkbox-input-size);
}

// hide error tooltip before a user interaction happened
.onyx-error-tooltip:has(&__input):not(:has(&__input:user-invalid)) .onyx-tooltip {
display: none;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ const meta: Meta<typeof OnyxCheckboxGroup> = {
argTypes: {
withCheckAll: { control: { type: "boolean" } },
},
decorators: [
(story) => ({
components: { story },
template: `
<div style="padding-left: 1rem;">
<story />
</div>`,
}),
],
}),
};

Expand All @@ -32,8 +41,8 @@ const DEMO_OPTIONS = [
label: "Invalid",
value: 7,
customError: {
shortMessage: "Example custom error",
longMessage: "This text might inform the users what they can do to fix the error.",
shortMessage: "Custom error",
longMessage: "Further explanation.",
},
},
] satisfies CheckboxGroupOption[];
Expand Down
Loading
Loading