fix: deep merge floatingUIOptions to preserve internal settings#2310
Conversation
|
@ysds is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
nperez0111
left a comment
There was a problem hiding this comment.
Hi @ysds, I appreciate you opening this pull request, but unfortunately we cannot take this as is. We definitely don't want to pull in lodash.merge as a dependency for what could be accomplished with another object spread
|
Thanks for the feedback! I'll update this PR to use nested spreads instead of The approach would look like this: const floatingUIOptions = useMemo<FloatingUIOptions>(
() => ({
...props.floatingUIOptions, // Pass through other options
useFloatingOptions: {
open: show,
onOpenChange: (open, _event, reason) => { ... },
placement,
middleware: [offset(10), shift(), flip()],
...props.floatingUIOptions?.useFloatingOptions, // User overrides
},
elementProps: {
style: { zIndex: 40 },
...props.floatingUIOptions?.elementProps, // User overrides
},
}),
[...]
);Does this approach look good to you? |
|
Yep, exactly what I would expect @ysds |
Use nested object spreads instead of lodash.merge to properly merge
floatingUIOptions properties. This prevents user-provided options like
`useFloatingOptions: { strategy: "fixed" }` from overwriting internal
settings such as `open`, `onOpenChange`, `placement`, and `middleware`.
Also fixes the type of floatingUIOptions in SuggestionMenuController
from UseFloatingOptions to FloatingUIOptions.
Closes TypeCellOS#2308
62bf888 to
c61cae0
Compare
|
Hi @nperez0111, |
nperez0111
left a comment
There was a problem hiding this comment.
Fantastic @ysds, exactly what I'd have done myself
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
Summary
Deep merge
floatingUIOptionsto preserve internal settings when custom options are passed.Rationale
Closes #2308
Changes
Uselodash.mergeinstead of spread operator for deep mergingfloatingUIOptionsin all controller componentsAddsatisfies FloatingUIOptionsto the first argument ofmerge()to maintain type inference for callback parameters (e.g.,onOpenChange)floatingUIOptionsin all controller components (no new dependencies required)floatingUIOptionsprop inSuggestionMenuControllerfromUseFloatingOptionstoFloatingUIOptionsImpact
Users can now pass partial
floatingUIOptionswithout losing the controller's internal default settings.Testing
Build passes.
Screenshots/Video
Checklist
Additional Notes