Skip to content

Commit

Permalink
feat(components): Remove/rename native event names on every component (
Browse files Browse the repository at this point in the history
…#1799)

Relates to #1603 

Remove/rename native event names on every component

---------

Co-authored-by: Jonathan Carle <[email protected]>
Co-authored-by: Lars Rickert <[email protected]>
  • Loading branch information
3 people authored Sep 17, 2024
1 parent 4b5d233 commit e6af99b
Show file tree
Hide file tree
Showing 24 changed files with 61 additions and 192 deletions.
14 changes: 14 additions & 0 deletions .changeset/hip-kids-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"sit-onyx": major
---

feat: Remove/Rename emits that collide with native event names

- OnyxInput, OnyxTextarea and OnyxStepper: Remove `focus` and `blur` event - Use `focusin`/`focusout` or `@focus.capture`/`@focus.blur` instead
- OnyxInput and OnyxTextarea: Remove Vue `change` emit - You will now receive the native `@change` event, but the value must now retrieved with with `$event.target.value` or use `@update:modelvalue`
- OnyxNavBar: Rename `appAreaClick` to `navigateToStart` and `backButtonClick` to `navigateBack`
- OnyxNavButton: Rename `click` to `navigate`, also the native MouseEvent is now passed as second parameter
- OnyxNavItem: Rename `click` to `navigate`
- OnyxRadioButton: Remove Vue `change` emit - You will now receive the native `@change` event, but the value must now retrieved with with `$event.target.value` or use `@update:modelvalue`
- OnyxSelectInput: Rename `click` to `inputClick`
- OnyxToastMessage: Remove Vue `click` emit - You will now always receive the native `@click` event, even when `clickable` prop is false/not set.
2 changes: 1 addition & 1 deletion apps/demo-app/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ watch(
:key="item.href"
v-bind="item"
:active="item.href === router.currentRoute.value.path"
@click="router.push"
@navigate="router.push"
/>

<template #contextArea>
Expand Down
8 changes: 0 additions & 8 deletions packages/sit-onyx/src/components/OnyxButton/OnyxButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ const props = withDefaults(defineProps<OnyxButtonProps>(), {
const { densityClass } = useDensity(props);
const emit = defineEmits<{
/**
* Emitted when the button is clicked (and is not disabled).
*/
click: [];
}>();
const rippleRef = ref<ComponentInstance<typeof OnyxRipple>>();
const rippleEvents = computed(() => rippleRef.value?.events ?? {});
</script>
Expand All @@ -44,7 +37,6 @@ const rippleEvents = computed(() => rippleRef.value?.events ?? {});
:type="props.type"
:aria-label="props.loading ? props.label : undefined"
:autofocus="props.autofocus"
@click="emit('click')"
v-on="rippleEvents"
>
<OnyxRipple v-if="!props.disabled && !props.loading" ref="rippleRef" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { HTMLAttributes } from "vue";
import { DENSITIES } from "../../composables/density";
import { expect, test } from "../../playwright/a11y";
import {
Expand All @@ -9,49 +10,53 @@ import { BUTTON_COLORS } from "../OnyxButton/types";
import OnyxIconButton from "./OnyxIconButton.vue";
import type { OnyxIconButtonProps } from "./types";

test("should behave correctly", async ({ mount }) => {
let clicks = 0;
test("should behave correctly", async ({ page, mount }) => {
const clickSpy: MouseEvent[] = [];
const setup = {
props: {
label: "trigger something",
icon: mockPlaywrightIcon,
} satisfies OnyxIconButtonProps,
on: {
click: () => clicks++,
},
onClick: (e) => clickSpy.push(e),
} satisfies OnyxIconButtonProps & HTMLAttributes,
};

// ARRANGE
const component = await mount(OnyxIconButton, setup);
const buttonElement = page.getByRole("button");

await test.step("clickable by default", async () => {
// ACT
await component.click();
await buttonElement.click();
// ASSERT
expect(clicks).toBe(1);
await expect(buttonElement).toBeEnabled();
expect(clickSpy).toHaveLength(1);
});

await test.step("not interactive when disabled ", async () => {
// ARRANGE
await component.update({ ...setup, props: { disabled: true } });
await component.update({ ...setup, props: { disabled: true } }); // ACT
// ACT
await buttonElement.click({ force: true });
// ASSERT
await expect(component).toBeDisabled();
await expect(buttonElement).toBeDisabled();
expect(clickSpy).toHaveLength(1);
});

await test.step("not interactive when loading ", async () => {
// ARRANGE
await component.update({ ...setup, props: { disabled: false, loading: true } });
// ASSERT
await expect(component).toBeDisabled();
await expect(buttonElement).toBeDisabled();
});

await test.step("clickable again ", async () => {
// ARRANGE
await component.update({ ...setup, props: { disabled: false, loading: false } });
// ACT
await component.click();
await buttonElement.click();
// ASSERT
expect(clicks).toBe(2);
await expect(buttonElement).toBeEnabled();
expect(clickSpy).toHaveLength(2);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ defineSlots<{
/** Slot for an custom icon. Will have no effect if property `icon` is passed. */
default(): unknown;
}>();
const emit = defineEmits<{
/** Emitted when the button is clicked (and is not disabled). */
click: [];
}>();
</script>

<template>
Expand All @@ -41,7 +36,6 @@ const emit = defineEmits<{
]"
:disabled="props.disabled || props.loading"
:autofocus="props.autofocus"
@click="emit('click')"
>
<OnyxLoadingIndicator v-if="props.loading" type="circle" />
<OnyxIcon v-else-if="props.icon" :icon="props.icon" />
Expand Down
21 changes: 0 additions & 21 deletions packages/sit-onyx/src/components/OnyxInput/OnyxInput.ct.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,27 +260,19 @@ test("should emit events", async ({ mount, makeAxeBuilder }) => {
const events = {
updateModelValue: [] as string[],
change: [] as string[],
focusCount: 0,
blurCount: 0,
};

// ARRANGE
const component = await mount(
<OnyxInput
label="Label"
onUpdate:modelValue={(value) => events.updateModelValue.push(value)}
onChange={(value) => events.change.push(value)}
onFocus={() => events.focusCount++}
onBlur={() => events.blurCount++}
/>,
);

// should not emit initial events
expect(events).toMatchObject({
updateModelValue: [],
change: [],
focusCount: 0,
blurCount: 0,
});

// ACT
Expand All @@ -298,19 +290,6 @@ test("should emit events", async ({ mount, makeAxeBuilder }) => {
await expect(inputElement).toHaveValue("Test");
expect(events).toMatchObject({
updateModelValue: ["T", "Te", "Tes", "Test"],
change: [],
focusCount: 1,
blurCount: 0,
});

// ACT
await inputElement.blur();
// ASSERT
expect(events).toMatchObject({
updateModelValue: ["T", "Te", "Tes", "Test"],
change: ["Test"],
focusCount: 1,
blurCount: 1,
});
});

Expand Down
20 changes: 0 additions & 20 deletions packages/sit-onyx/src/components/OnyxInput/OnyxInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,6 @@ const emit = defineEmits<{
* Emitted when the current value changes.
*/
"update:modelValue": [value: string];
/**
* Emitted when the current value changes but only when the input is blurred.
*/
change: [value: string];
/**
* Emitted when the input is focussed.
*/
focus: [];
/**
* Emitted when the input is blurred.
*/
blur: [];
/**
* Emitted when the validity state of the input changes.
*/
Expand All @@ -53,11 +41,6 @@ const value = computed({
set: (value) => emit("update:modelValue", value),
});
const handleChange = (event: Event) => {
const inputValue = (event.target as HTMLInputElement).value;
emit("change", inputValue);
};
const patternSource = computed(() => {
if (props.pattern instanceof RegExp) return props.pattern.source;
return props.pattern;
Expand Down Expand Up @@ -94,9 +77,6 @@ const patternSource = computed(() => {
:maxlength="props.maxlength"
:aria-label="props.hideLabel ? props.label : undefined"
:title="props.hideLabel ? props.label : undefined"
@change="handleChange"
@focus="emit('focus')"
@blur="emit('blur')"
/>
<!-- eslint-enable vuejs-accessibility/no-autofocus -->
</div>
Expand Down
8 changes: 0 additions & 8 deletions packages/sit-onyx/src/components/OnyxLink/OnyxLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ const props = withDefaults(defineProps<OnyxLinkProps>(), {
withExternalIcon: "auto",
});
const emit = defineEmits<{
/**
* Emitted when the link is opened (via click or keyboard).
*/
click: [];
}>();
defineSlots<{
/**
* Link label.
Expand All @@ -32,7 +25,6 @@ const { t } = injectI18n();
:href="props.href"
:target="props.target"
:rel="props.target === '_blank' ? 'noreferrer' : undefined"
@click="emit('click')"
>
<slot></slot>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,6 @@ import type { OnyxNavAppAreaProps } from "./types";
const props = defineProps<OnyxNavAppAreaProps>();
const emit = defineEmits<{
/**
* Emitted when the app area (where logo and app name are placed) is clicked.
* Usually the user is redirected to the home page.
*/
click: [];
}>();
defineSlots<{
/**
* Optional slot to override the content.
Expand All @@ -26,7 +18,7 @@ const buttonLabel = computed(() => props.label ?? t.value("navigation.goToHome")
</script>

<template>
<button type="button" class="onyx-nav-app-area" :aria-label="buttonLabel" @click="emit('click')">
<button type="button" class="onyx-nav-app-area" :aria-label="buttonLabel">
<slot>
<!--
the width/height here is only to prevent layout shifts on initial load.
Expand Down
16 changes: 8 additions & 8 deletions packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.ct.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,33 +90,33 @@ test("Screenshot tests (mobile)", async ({ mount, page }) => {

const component = await mount(
<OnyxNavBar appName="App name" logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}>
<OnyxNavButton href="#1" label="Item 1" onClick={(href) => clickEvents.push(href)} />
<OnyxNavButton href="#2" label="Item 2" onClick={(href) => clickEvents.push(href)}>
<OnyxNavButton href="#1" label="Item 1" onNavigate={(href) => clickEvents.push(href)} />
<OnyxNavButton href="#2" label="Item 2" onNavigate={(href) => clickEvents.push(href)}>
Item 2
<OnyxBadge color="warning" dot />
<template v-slot:children>
<OnyxNavItem
label="Nested item 1"
href="#2-1"
onClick={(href) => clickEvents.push(href)}
onNavigate={(href) => clickEvents.push(href)}
/>
<OnyxNavItem
label="Nested item 2"
href="#2-2"
active
onClick={(href) => clickEvents.push(href)}
onNavigate={(href) => clickEvents.push(href)}
/>
<OnyxNavItem
label="Nested item 3"
href="#2-3"
onClick={(href) => clickEvents.push(href)}
onNavigate={(href) => clickEvents.push(href)}
/>
</template>
</OnyxNavButton>
<OnyxNavButton
href="https://onyx.schwarz"
label="Item 3"
onClick={(href) => clickEvents.push(href)}
onNavigate={(href) => clickEvents.push(href)}
/>

<template v-slot:mobileActivePage>Nested item 2</template>
Expand Down Expand Up @@ -241,8 +241,8 @@ test("should behave correctly", async ({ mount }) => {
appName="App name"
logoUrl={MOCK_PLAYWRIGHT_LOGO_URL}
withBackButton
onAppAreaClick={() => appAreaClickEvents++}
onBackButtonClick={() => backButtonClickEvents++}
onNavigateToStart={() => appAreaClickEvents++}
onNavigateBack={() => backButtonClickEvents++}
/>,
);

Expand Down
8 changes: 4 additions & 4 deletions packages/sit-onyx/src/components/OnyxNavBar/OnyxNavBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ const emit = defineEmits<{
* Emitted when the app area (where logo and app name are placed) is clicked.
* Usually the user should be redirected to the home page then.
*/
appAreaClick: [];
navigateToStart: [event: MouseEvent];
/**
* Emitted when the back button is clicked.
*/
backButtonClick: [];
navigateBack: [event: MouseEvent];
}>();
const slots = defineSlots<{
Expand Down Expand Up @@ -114,7 +114,7 @@ defineExpose({
:logo-url="props.logoUrl"
:label="props.appAreaLabel"
@click="
emit('appAreaClick');
emit('navigateToStart', $event);
isBurgerOpen = false;
"
>
Expand All @@ -127,7 +127,7 @@ defineExpose({
:label="t('navigation.goBack')"
:icon="chevronLeftSmall"
color="neutral"
@click="emit('backButtonClick')"
@click="emit('navigateBack', $event)"
/>

<template v-if="slots.default">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ import { type OnyxMenuItemProps } from "./types";
const props = defineProps<OnyxMenuItemProps>();
const emit = defineEmits<{
/**
* Emitted when the menu item is clicked (via click or keyboard).
*/
click: [];
}>();
const {
elements: { listItem, menuItem },
} = createMenuItems();
Expand All @@ -37,7 +30,6 @@ const {
disabled: !props.href && props.disabled,
})
"
@click="emit('click')"
>
<slot></slot>
</component>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ provide(
:label="props.label"
:href="props.href"
:active="props.active"
@click="emit('click', $event)"
@navigate="emit('click', $event)"
>
<slot></slot>

Expand Down
Loading

0 comments on commit e6af99b

Please sign in to comment.