fix(menu): merge props before normalizing in getTriggerItemProps#2695
fix(menu): merge props before normalizing in getTriggerItemProps#2695nikparo wants to merge 2 commits intochakra-ui:mainfrom
Conversation
🦋 Changeset detectedLatest commit: fe02ca7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 78 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
nikparo
force-pushed
the
fix/separate-menu-prop-handling
branch
from
0102354 to
fe02ca7
Compare
|
Hey @nikparo, thanks for the detailed write-up — the bug you've identified is real, but I think the fix belongs in your Lit adapter PR (#2698) rather than in the menu connect logic. Here's why: for all currently supported frameworks (React, Solid, Vue, Svelte, Preact, Vanilla), Instead of changing the menu internals, I'd suggest providing a custom // packages/frameworks/lit/src/merge-props.ts
import { mergeProps as coreMergeProps } from "@zag-js/core"
export function mergeProps<T extends Record<string, any>>(
...args: Array<T | undefined>
): T {
// Convert @-prefixed event handlers to on- prefix
// so core mergeProps chains them correctly, then convert back.
const normalized = args.map((props) => {
if (!props) return props
const result: Record<string, any> = {}
for (const key in props) {
if (key.startsWith("@")) {
result[`on${key.slice(1)}`] = props[key]
} else {
result[key] = props[key]
}
}
return result as T
})
const merged = coreMergeProps(...normalized)
// Convert back to @-prefixed
const result: Record<string, any> = {}
for (const key in merged) {
if (key.startsWith("on") && typeof merged[key] === "function") {
result[`@${key.slice(2).toLowerCase()}`] = merged[key]
} else {
result[key] = merged[key]
}
}
return result as T
}This keeps the framework-specific concern in the framework adapter, which is the whole point of the adapter pattern. Every other framework adapter (React, Solid, etc.) re-exports Let me know if you run into issues with this approach in #2698! |
|
Closing — see comment above. The fix belongs in the Lit adapter PR (#2698). |
2 participants
Closes #
📝 Description
The menu
getItemPropsandgetTriggerPropsreturn normalized props, that then get merged using the coremergePropsfunction that does not take normalization into account. This is a problem for e.g. Lit, where the event handler prefix is@rather thanon. To properly merge these sets of props in theconnectfunction we need to separate handling of framework-specific and framework-agnostic prop handling.⛳️ Current behavior (updates)
If the
normalizePropsfunction does not useonprefix for event handlers, only the latter is kept.🚀 New behavior
Event handlers are properly merged, as prop normalization happens after the merge.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information