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

chore: replace redundant useManagedState with Vue's defineModel #2529

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
11 changes: 11 additions & 0 deletions .changeset/three-hornets-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@sit-onyx/headless": patch
"sit-onyx": major
---

chore: replace redundant useManagedState with defineModel

The changes are mostly internal, but the typings were of `OnyxSelect` were improved:

- The `modelValue` now infers a specific subtype of `SelectOptionValue` and the `options` values must match.
- `withSearch`: Filtering of the options will not automatically disabled anymore when `searchTerm` is bound. Instead `noFilter` must be set.
16 changes: 9 additions & 7 deletions apps/demo-app/src/components/SelectDemo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { normalizedIncludes, OnyxSelect, type SelectOption } from "sit-onyx";
import { computed, ref } from "vue";

defineProps<{
selectOptions: SelectOption[];
selectOptions: SelectOption<string>[];
useSkeleton: boolean;
}>();

const groupedSelectOptions: SelectOption[] = [
const groupedSelectOptions = [
{ value: "cat", label: "Cat", group: "Land" },
{ value: "dog", label: "Dog", group: "Land" },
{ value: "tiger", label: "Tiger", group: "Land" },
Expand All @@ -18,22 +18,22 @@ const groupedSelectOptions: SelectOption[] = [
{ value: "eel", label: "Eel", group: "Water" },
{ value: "falcon", label: "Falcon", group: "Air" },
{ value: "owl", label: "Owl", group: "Air" },
];
] satisfies SelectOption[];

const selectState = ref<string>();
const groupedSelectState = ref<string>();
const multiselectState = ref<string[]>();

const lazyLoadedState = ref(15);
const lazyLoadedLength = ref(10);
const lazyLoadedOptions = computed<SelectOption[]>(() =>
const lazyLoadedOptions = computed<SelectOption<number>[]>(() =>
Array.from({ length: lazyLoadedLength.value }, (_, value) => ({
value,
label: `Lazy option ${value}`,
})),
);

const filteredState = ref(3);
const filteredState = ref("3");
const filterSearchTerm = ref("");
const filterBase = [
{ value: "0", label: "Option Zero" },
Expand All @@ -43,8 +43,10 @@ const filterBase = [
{ value: "4", label: "Option Four" },
{ value: "5", label: "Option Five" },
];
const filterValueLabel = computed(() => filterBase[filteredState.value].label);
const filteredOptions = computed<SelectOption[]>(() =>
const filterValueLabel = computed(
() => filterBase.find(({ value }) => value === filteredState.value)?.label,
);
const filteredOptions = computed(() =>
filterBase.filter(
({ value, label }) =>
normalizedIncludes(label, filterSearchTerm.value) || value === filterSearchTerm.value,
Expand Down
2 changes: 1 addition & 1 deletion apps/demo-app/src/views/HomeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const selectOptions = [
"Melon",
"Raspberry",
"Strawberry",
].map<SelectOption>((option) => ({ value: option.toLowerCase(), label: option }));
].map((option) => ({ value: option.toLowerCase(), label: option }));

const minimalSelectOptions = selectOptions.slice(0, 3);

Expand Down
1 change: 1 addition & 0 deletions apps/demo-app/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"include": ["src/**/*"],
"compilerOptions": {
"rootDir": "./src",
"lib": ["DOM", "DOM.Iterable", "ES2022"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this change needed? Same question for the playground

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Honestly, I don't know why it is needed now.
Without it, I am getting errors:
Screenshot 2025-01-17 at 08 58 30

The errors make sense, but why only after all these seemingly unrelated changes?
My assumption is, that the Vue compiler can handle the updated typings better and doesn't bail out (as early)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm shouldn't the headless code be inlined? Maybe this has something to do with the update to Vite 6 in #2530 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

JavaScript is inlined, the typings are not inlined afaik

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it is related, because there are no issues on main

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should find out why and fix this, otherwise propably all projects using onyx will have build errors...

"baseUrl": "."
}
}
1 change: 1 addition & 0 deletions apps/playground/tsconfig.app.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"compilerOptions": {
"composite": true,
"rootDir": "./src",
"lib": ["ES2022", "DOM", "DOM.Iterable"],
"baseUrl": "."
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import type { Arrayable } from "vitest";
import { toValue, type Ref } from "vue";
import type { MaybeReactiveSource } from "../../utils/types";
import { toValue, type MaybeRefOrGetter, type Ref } from "vue";
import type { Arrayable } from "../../utils/types";
import { useGlobalEventListener } from "./useGlobalListener";

export type UseOutsideClickOptions = {
/**
* HTML element of the component where clicks should be ignored
*/
inside: MaybeReactiveSource<Arrayable<HTMLElement | undefined>>;
inside: MaybeRefOrGetter<Arrayable<HTMLElement | undefined>>;
/**
* Callback when an outside click occurred.
*/
Expand Down
6 changes: 2 additions & 4 deletions packages/headless/src/utils/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { ComputedRef, MaybeRefOrGetter } from "vue";

/**
* Adds the entry with the key `Key` and the value of type `TValue` to a record when it is defined.
* Then the entry is either undefined or exists without being optional.
Expand All @@ -25,6 +23,6 @@ export type IsArray<TValue, TMultiple extends boolean = false> = TMultiple exten
: TValue;

/**
* Type for any kind of ref source. Preferably used in combination with vue's `toValue` method
* A type that can be wrapped in an array.
*/
export type MaybeReactiveSource<T> = MaybeRefOrGetter<T> | ComputedRef<T>;
export type Arrayable<T> = T | Array<T>;
9 changes: 0 additions & 9 deletions packages/sit-onyx/.storybook/managed.ts

This file was deleted.

9 changes: 4 additions & 5 deletions packages/sit-onyx/.storybook/preview.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createPreview } from "@sit-onyx/storybook-utils";
import { createPreview, withVModelDecorator } from "@sit-onyx/storybook-utils";
import { setup, type Preview } from "@storybook/vue3";
import { createToastProvider, TOAST_PROVIDER_INJECTION_KEY } from "../src";
import docsTemplate from "./docs-template.mdx";
Expand All @@ -12,9 +12,8 @@ import { a11yTags } from "../src/a11yConfig";
import "../src/styles/index.scss";
import "./docs-template.scss";
import { enhanceFormInjectedSymbol } from "./formInjected";
import { enhanceManagedSymbol } from "./managed";
import brandImage from "./public/onyx-logo-long.svg";
import { withOnyxVModelDecorator } from "./vModel";

const enabledRules = getRules(a11yTags).map((ruleMetadata) => ({
id: ruleMetadata.ruleId,
enabled: true,
Expand All @@ -24,7 +23,7 @@ const axeConfig: Spec = { rules: enabledRules };

const basePreview = createPreview(
{
argTypesEnhancers: [enhanceManagedSymbol, enhanceFormInjectedSymbol],
argTypesEnhancers: [enhanceFormInjectedSymbol],
parameters: {
docs: {
page: docsTemplate,
Expand All @@ -42,7 +41,7 @@ const basePreview = createPreview(
globalTypes: {
...onyxThemeGlobalType,
},
decorators: [withOnyxTheme, withOnyxVModelDecorator],
decorators: [withOnyxTheme, withVModelDecorator()],
},
{
brandImage,
Expand Down
16 changes: 0 additions & 16 deletions packages/sit-onyx/.storybook/vModel.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,22 @@ defineOptions({ inheritAttrs: false });
const props = defineProps<OnyxMiniSearchProps>();

const emit = defineEmits<{
/**
* Emitted when the current search value changes.
*/
"update:modelValue": [input: string];
/**
* Emitted when the clear button is clicked.
*/
clear: [];
}>();

/**
* Current input/search value.
*/
const modelValue = defineModel<string>({ default: "" });

const { rootAttrs, restAttrs } = useRootAttrs();
const { densityClass } = useDensity(props);
const { t } = injectI18n();
const input = ref<HTMLInputElement>();

/**
* Current value (with getter and setter) that can be used as "v-model" for the native input.
*/
const value = computed({
get: () => props.modelValue,
set: (value) => emit("update:modelValue", value ?? ""),
});

const placeholder = computed(() => t.value("select.searchPlaceholder"));

defineExpose({
Expand All @@ -54,7 +47,7 @@ defineExpose({
>
<input
ref="input"
v-model="value"
v-model="modelValue"
class="onyx-mini-search__input onyx-text"
:placeholder="placeholder"
type="text"
Expand Down
4 changes: 0 additions & 4 deletions packages/sit-onyx/src/components/OnyxMiniSearch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,4 @@ export type OnyxMiniSearchProps = DensityProp & {
* (Aria) label of the input.
*/
label: string;
/**
* Current input/search value.
*/
modelValue?: string;
};
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
<script setup lang="ts" generic="TValue extends SelectOptionValue = SelectOptionValue">
import { createMenuButton } from "@sit-onyx/headless";
import { computed, toRef } from "vue";
import { MANAGED_SYMBOL, useManagedState } from "../../../../composables/useManagedState";
import { computed } from "vue";
import type { SelectOptionValue } from "../../../../types";
import type { OnyxFlyoutMenuProps } from "./types";

const props = withDefaults(defineProps<OnyxFlyoutMenuProps>(), {
open: MANAGED_SYMBOL,
trigger: "hover",
});

const emit = defineEmits<{
"update:open": [isOpen: boolean];
}>();

const { state: isExpanded } = useManagedState(
toRef(() => props.open),
false,
(newVal) => emit("update:open", newVal),
);
/**
* If the flyout is expanded or not.
*/
const isExpanded = defineModel<boolean>("open", { default: false });

const slots = defineSlots<{
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import type { ManagedProp } from "../../../../composables/useManagedState";

export type OnyxFlyoutMenuProps = {
/**
* If the flyout is expanded or not.
* If `undefined`, the state will be managed internally.
*/
open?: ManagedProp<boolean>;
/**
* If the flyout is expanded on click or hover.
* The default value is 'hover' which will expand the flyout on hover.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ const slots = defineSlots<{

const { t } = injectI18n();

const emit = defineEmits<{
"update:mobileChildrenOpen": [isOpen: boolean];
}>();
const mobileChildrenOpen = defineModel<boolean>("mobileChildrenOpen", { default: false });
</script>

<template>
Expand All @@ -46,7 +44,7 @@ const emit = defineEmits<{
mode="plain"
color="neutral"
:icon="arrowSmallLeft"
@click="emit('update:mobileChildrenOpen', false)"
@click="mobileChildrenOpen = false"
/>

<slot
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<script lang="ts" setup>
import chevronRightSmall from "@sit-onyx/icons/chevron-right-small.svg?raw";
import { computed, inject, toRef } from "vue";
import { MANAGED_SYMBOL, useManagedState } from "../../../../composables/useManagedState";
import { computed, inject } from "vue";
import { useMoreListChild } from "../../../../composables/useMoreList";
import OnyxExternalLinkIcon from "../../../OnyxExternalLinkIcon/OnyxExternalLinkIcon.vue";
import OnyxIcon from "../../../OnyxIcon/OnyxIcon.vue";
Expand All @@ -12,18 +11,13 @@ import type { OnyxNavButtonProps } from "./types";
const props = withDefaults(defineProps<OnyxNavButtonProps>(), {
active: false,
withExternalIcon: "auto",
mobileChildrenOpen: MANAGED_SYMBOL,
});

const emit = defineEmits<{
/**
* Emitted when the nav button is clicked (via click or keyboard).
*/
navigate: [href: string, event: MouseEvent];
/**
* Emitted when the mobile children are open or closed.
*/
"update:mobileChildrenOpen": [isOpen: boolean];
}>();

const slots = defineSlots<{
Expand All @@ -37,19 +31,18 @@ const slots = defineSlots<{
children?(): unknown;
}>();

/**
* Controls the open state for the mobile children.
*/
const mobileChildrenOpen = defineModel<boolean>("mobileChildrenOpen", { default: false });

const isMobile = inject(
MOBILE_NAV_BAR_INJECTION_KEY,
computed(() => false),
);
const hasChildren = computed(() => !!slots.children);
const { componentRef, isVisible } = useMoreListChild(NAV_BAR_MORE_LIST_INJECTION_KEY);

const { state: mobileChildrenOpen } = useManagedState(
toRef(() => props.mobileChildrenOpen),
false,
(newVal) => emit("update:mobileChildrenOpen", newVal),
);

const handleParentClick = (event: MouseEvent) => {
if (isMobile?.value && hasChildren.value && !mobileChildrenOpen.value) {
mobileChildrenOpen.value = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import type { ManagedProp } from "../../../../composables/useManagedState";
import type { OnyxExternalLinkIcon } from "../../../OnyxExternalLinkIcon/types";

export type OnyxNavButtonProps = OnyxExternalLinkIcon & {
/**
* Controls the open state for the mobile children.
* Is managed internally if not provided.
*/
mobileChildrenOpen?: ManagedProp<boolean>;
/**
* Label to show inside the Nav item.
* You can use the `default` slot to display custom content.
Expand Down
Loading
Loading