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.

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

feat(components): Remove/rename native event names on every component #1799

Merged
merged 35 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
669d099
Remove/rename click event on every component
MajaZarkova Aug 26, 2024
918a684
Update event names
MajaZarkova Aug 26, 2024
fed93ee
Fix tests
MajaZarkova Aug 26, 2024
4f7c44a
Remove change events
MajaZarkova Aug 27, 2024
85e4fe2
Rename focus and blur events
MajaZarkova Aug 27, 2024
437e40f
Adjust tests
MajaZarkova Aug 27, 2024
8b68b56
Add type events
MajaZarkova Aug 28, 2024
a9e0237
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
MajaZarkova Aug 28, 2024
078d2e2
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
MajaZarkova Aug 28, 2024
1515883
Rename click event on OnyxSelectInput
MajaZarkova Aug 29, 2024
9e121c9
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
MajaZarkova Sep 4, 2024
874f945
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
MajaZarkova Sep 4, 2024
0a74bbf
Merge with main
MajaZarkova Sep 5, 2024
6dd88a7
Remove focus and blur events from OnyxInput
MajaZarkova Sep 5, 2024
60f9e38
Remove focus and blur event from OnyxStepper
MajaZarkova Sep 5, 2024
b9483a6
Remove focus and blur event from OnyxTextarea
MajaZarkova Sep 5, 2024
d9a7fe1
Remove click event from OnyxToastMessage
MajaZarkova Sep 5, 2024
c1172da
Rename navItemClick and navButtonClick events to navigate
MajaZarkova Sep 5, 2024
7c5e350
Merge branch 'main' of https://github.com/SchwarzIT/onyx into feat/16…
JoCa96 Sep 11, 2024
7fa466b
review: rename events
JoCa96 Sep 11, 2024
ad82020
Merge branch 'main' of https://github.com/SchwarzIT/onyx into feat/16…
JoCa96 Sep 11, 2024
df40d77
fix test
JoCa96 Sep 12, 2024
9303360
review: update clickable description
JoCa96 Sep 12, 2024
706dd3e
Update packages/sit-onyx/src/components/OnyxSelectInput/OnyxSelectInp…
JoCa96 Sep 12, 2024
82b696a
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
JoCa96 Sep 13, 2024
b5bef99
Merge branch 'feat/1603-Remove/rename-native-event-names' of https://…
JoCa96 Sep 13, 2024
816e836
fix prettier issue
JoCa96 Sep 13, 2024
606186f
review: update icon button test
JoCa96 Sep 13, 2024
47cdf0d
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
JoCa96 Sep 16, 2024
69da60f
docs(changeset): feat: Remove/Rename emits that collide with native e…
JoCa96 Sep 16, 2024
53183eb
Merge branch 'feat/1603-Remove/rename-native-event-names' of https://…
JoCa96 Sep 16, 2024
421bba7
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
JoCa96 Sep 16, 2024
1e9b649
Merge branch 'main' into feat/1603-Remove/rename-native-event-names
larsrickert Sep 16, 2024
4542e4b
Update .changeset/hip-kids-decide.md
JoCa96 Sep 17, 2024
bacbef4
Merge branch 'main' of https://github.com/SchwarzIT/onyx into feat/16…
JoCa96 Sep 17, 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
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
JoCa96 marked this conversation as resolved.
Show resolved Hide resolved

- 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 });
Dismissed Show dismissed Hide dismissed
// 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];
larsrickert marked this conversation as resolved.
Show resolved Hide resolved
}>();

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
Loading