Changes to public composables #2425
Conversation
…s (like in vBToggle)
…he trigger to other hide emits.
…nents fix(BTooltip)!: change prop content to body to align with other components
feat(usePopoverController): add slots
…now. Add slots, rename pos -> position
|
|
|
@VividLemon @dwgray any ideas why the docs build hangs forever? |
… into flatten-composables
packages/bootstrap-vue-next/src/composables/useToastController/index.ts
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/plugins/toastController/index.ts
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/plugins/toastController/index.ts
Outdated
Show resolved
Hide resolved
packages/bootstrap-vue-next/src/plugins/toastController/index.ts
Outdated
Show resolved
Hide resolved
We'll figure it out after I'm done reviewing. Still not done yet, but I've submitted the first set of changes I'd like to see |
commit: |
I was thinking of creating a useToggle that can control all the useShowHide components, that would make useModal kinda reduntant. Thoughts? The documentation build was due to setInterval in script setup. Which is weird because I didn't add them 😄 |
|
@VividLemon I'm thinking of removing the support for "component" their slots don't work at the moment, we need to support all the slots for that to work, now we just render what ever is in modal.slots for example. I couldn't find a way to list components available slots, other that just putting them to a variable, so I did the current version. Only thing I think it's useful for is to have a custom default for the bmodal and you don't want to list them in the function args. -- Added later: I figured why the slots of |
There was a problem hiding this comment.
How do you share many same props between two instances of show?
const mySharedProps = computed(() => ({ foo: bar.value, buzz: aef.value }))
show({ ...mySharedProps.value, a: 1 })
show({ ...mySharedProps.value, b: 2 })This would required show(computed(() => ({ ...
Is this valid syntax? Ideally both show({ title: ref() }) and show(computed(() => ({ title: ref() }))) would be valid
They should all work, I haven't tested all. But the { var: ref() } part is only typed for title and body at the moment and it goest through toValue inside the watch. Maybe I'll loop all the props to be able to be refs... I'll test next week |
show(ref({ foo: 'bar' })
show({foo: ref('bar') })
show(() => ({ foo: props.bar }))
show(computed(() => ({ ...computedValue.value }))I think for simplicity just the root should be How this would work with values to be updated, I'm not sure yet.
|
export type MaybeRefOrGetter<T = any> = MaybeRef<T> | ComputedRef<T> | (() => T);it's not readonly |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/bootstrap-vue-next/src/components/BToast/BToast.vue (1)
50-50: Remove commented-out code.This commented-out code is no longer needed as it's been replaced by the new implementation.
- <!-- <BCloseButton class="ms-auto" @click="hide('close')" /> -->packages/bootstrap-vue-next/src/components/BAlert/BAlert.vue (2)
195-196: Resolve the TODO comment about unused 'solid' propThere's a TODO comment indicating the 'solid' prop is never used, yet it's defined in the props (line 138). This should be addressed by either implementing the usage of this prop or removing it.
-// TODO solid is never used +// Apply solid styles to alert if needed const countdownLength = computed(() => typeof modelValue.value === 'boolean' ? 0 : modelValue.value )Or remove the prop if truly not needed:
- solid: false,
231-240: Consider implementing color classes or remove commented codeThere's commented out code related to color classes which suggests incomplete implementation. Either implement the color variant classes functionality or remove the commented code to keep the codebase clean.
-// const colorClasses = useColorVariantClasses(props) const computedClasses = computed(() => [ - // colorClasses.value, { [`alert-${props.variant}`]: props.variant !== null, 'alert-dismissible': props.dismissible && !(slots.close || props.closeContent), 'show': isVisible.value, 'fade': !computedNoAnimation.value, }, ])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bootstrap-vue-next/src/components/BAlert/BAlert.vue(1 hunks)packages/bootstrap-vue-next/src/components/BToast/BToast.vue(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (20)
packages/bootstrap-vue-next/src/components/BToast/BToast.vue (9)
14-14: Consistent display style added to root element.Adding
style="display: block"to the root element ensures consistent rendering across browsers, aligning with similar changes made to other components like BModal and BDropdown.
25-29: Improved slot binding with shared context.The title slot now receives reactive state and control methods via the
sharedSlotsbinding, providing slot content with access to component state and methods liketoggle,show, andhide.
30-49: Enhanced close button customization.The close button implementation has been significantly improved to:
- Conditionally render either a BButton (for custom content) or a BCloseButton (default)
- Support customization via new props (closeClass, closeContent, closeVariant, closeLabel)
- Maintain the ms-auto class for proper alignment
This provides much more flexibility for customizing the close button while maintaining accessibility.
53-86: Improved toast body structure with flex layout.The toast body has been restructured to:
- Use a flex container for better alignment
- Conditionally render a close button in the body when there's no header
- Apply the same customization options as the header close button
- Use consistent styles and behavior
This improves the layout consistency and provides better support for various usage patterns.
108-109: Updated type imports for better standardization.Type imports have been updated to use the standardized types from ComponentEmits and ComponentSlots, aligning with the broader refactoring effort to standardize component interfaces across the library.
124-127: Added new props for close button customization.New props with sensible defaults:
closeClass: for additional stylingcloseContent: for custom text/contentcloseLabel: for screen reader accessibility (defaulting to "Close")closeVariant: for button styling (defaulting to "secondary")These provide more flexibility while maintaining backward compatibility.
176-178: Updated emit and slot definitions for better type safety.The component now uses the standardized type definitions for emits and slots, improving type safety and consistency across the component library.
265-272: Added sharedSlots computed property for consistent slot API.This computed property creates a unified object with methods and state to pass to slots, making the slot API consistent and predictable. It provides slots with access to:
- Control methods: toggle, show, hide
- State information: id, visible, active
This follows the pattern being adopted across components for more consistent slot interfaces.
295-297: Added CSS for custom close button.The
.btn-close-customclass ensures proper styling for the custom close button in the toast body, using Bootstrap CSS variables for consistency with the design system.packages/bootstrap-vue-next/src/components/BAlert/BAlert.vue (11)
2-6: Good use of conditional rendering for the Transition wrapperThe transition wrapper now properly conditionally renders based on
renderRef || contentShowingand uses dynamic binding for transition properties. Theappearattribute correctly uses visibility props which provides better initial animation control.
7-17: Improved accessibility with conditional ARIA attributesThe alert container now uses
v-showinstead ofv-iffor better performance when toggling visibility. The addition of dynamicid,tabindex="0", and conditional ARIA attributes based on visibility and status mode significantly improves accessibility compliance.
18-48: Structured header component with flexible content optionsThe header implementation now provides better structure with conditional rendering, proper slot binding, and flexible content options. The close button handling is well-implemented with both custom and default options.
49-83: Well-structured body content with improved slot handlingThe body content implementation uses a computed tag based on link status and provides good flexibility for content via slots or props. The duplicate close button logic for when there's no header is a good addition for consistent UX.
84-95: Progress bar implementation with proper props bindingThe progress bar is correctly conditionally rendered based on modelValue type and progressProps. All necessary props are properly passed through to the BProgress component.
182-193: Good integration of useShowHide composableThe integration of the useShowHide composable centralizes visibility logic, provides consistent show/hide behavior, and improves maintainability. The destructured properties are used effectively throughout the component.
242-252: Well-implemented modelValue watcher with event handlingThe watch on modelValue correctly handles numeric values by emitting events and restarting the countdown. The event cancellation handling is properly implemented with preventable show events.
254-261: Proper lifecycle management in isActive watcherThe watch on isActive correctly handles the alert lifecycle by hiding it when the countdown is done and not paused. It properly resets the modelValue and stops the countdown.
262-269: Comprehensive slot props for consistent slot behaviorThe sharedSlots computed property provides a consistent set of slot props across all slots, making the component more predictable and easier to use with custom slot content.
271-279: Complete set of exposed methods for external controlThe defineExpose call includes all necessary methods for external control of the component, providing a comprehensive API for parent components to interact with the alert.
283-285: Performance improvement for progress bar transitionsDisabling the progress bar transition improves performance during countdown by preventing unnecessary animation overhead.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/docs/src/docs/migration-guide.md (1)
730-730: Remove redundant blank lines
There are multiple consecutive blank lines here, which trigger MD012 in markdownlint. Please collapse to a single blank line to keep the document clean.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
730-730: Multiple consecutive blank lines
Expected: 1; Actual: 2(MD012, no-multiple-blanks)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/docs/src/docs/migration-guide.md(2 hunks)packages/bootstrap-vue-next/src/components/BAccordion/BAccordionItem.vue(2 hunks)packages/bootstrap-vue-next/src/components/BDropdown/BDropdown.vue(3 hunks)packages/bootstrap-vue-next/src/components/BPopover/BPopover.vue(9 hunks)packages/bootstrap-vue-next/src/composables/useShowHide.ts(11 hunks)packages/bootstrap-vue-next/src/types/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/bootstrap-vue-next/src/components/BAccordion/BAccordionItem.vue
- packages/bootstrap-vue-next/src/components/BDropdown/BDropdown.vue
- packages/bootstrap-vue-next/src/components/BPopover/BPopover.vue
- packages/bootstrap-vue-next/src/types/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/composables/useShowHide.ts (3)
packages/bootstrap-vue-next/src/composables/useModal/index.ts (2)
hide(53-55)show(50-52)packages/bootstrap-vue-next/src/utils/keys.ts (1)
globalShowHideStorageInjectionKey(175-176)packages/bootstrap-vue-next/src/utils/dom.ts (1)
isVisible(32-47)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/src/docs/migration-guide.md
730-730: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
🔇 Additional comments (26)
apps/docs/src/docs/migration-guide.md (3)
603-603: Consistent prop rename for BPopover section
The migration guide now correctly notes that thecontentprop has been renamed tobodyin theBPopovercomponent. This aligns with the code changes in the props and tests. Please verify that any example code or fragments underBPopoverreferencebodyinstead ofcontent.
726-726: Update v-html deprecation reference in BTooltip
A deprecation note for thehtmlprop has been added underBTooltippointing to the#v-htmlsection. Ensure that the anchor link is valid and that the v-html guidance covers all tooltip-related HTML deprecations.
728-728: Consistent prop rename for BTooltip section
The guide now reflects thecontent→bodyrename forBTooltip. Confirm that code examples, demos, and the API reference forBTooltipall usebodyand not the deprecatedcontent.packages/bootstrap-vue-next/src/composables/useShowHide.ts (23)
178-180: Promise-based control flow for state managementThese private variables manage promise resolution state for the new asynchronous API. This aligns with the PR's objective to introduce promise-based control flows for composables.
181-185: Improved show function with promise-based returnThe
showfunction now returns a Promise, allowing for better async control flow. The early return cases are correctly handled, preventing redundant operations when the component is already visible.
186-189: Promise initialization pattern for show operationThis creates a new promise and stores the resolver function for later resolution, which is a clean pattern for async operations.
204-205: Promise resolution for prevented show eventsProperly handles rejection cases by resolving the promise with
falsewhen show events are prevented.
214-224: Simplified animation handling logicThe condition for animation has been simplified to check both local animation state and delay configuration in one statement, making the code more maintainable.
238-242: Fixed delay property access and promise returnCorrectly handles the different types of delay prop (number or object) and returns the promise at the end of the function.
244-262: Promise-based hide function with trigger contextThe
hidefunction now:
- Returns a Promise
- Records the trigger that caused the hide operation
- Properly handles early returns and promise resolution for prevented events
- Provides clearer error messages with trigger context
This implementation aligns with the PR objective to improve lifecycle management and event handling.
267-268: Conditional trigger event emissionAdded control over trigger event emission through the
noTriggerEmitparameter, which prevents redundant events when hide is called programmatically.
273-282: Improved hide prevention with trigger contextEnhances the hide prevention logic by including the trigger information in the event and handling promise resolution appropriately.
283-283: Early focus trap deactivationDisables the focus trap as soon as hide begins, improving accessibility by releasing focus before animation completes.
308-309: Promise return from hide functionProperly returns the promise at the end of the hide function, allowing consumers to await the completion of the hide operation.
313-324: Promise-based toggle functionReimplemented the toggle function to return a Promise, properly handling event prevention and delegating to either show or hide as appropriate. The
resolveOnHideparameter provides flexibility for controlling when the promise resolves.
325-337: Added trigger-specific toggle functionSeparates the toggle logic into two functions:
toggle- For programmatic control with promise returntriggerToggle- For event-based toggles from UI elementsThis separation improves the API by providing specialized functions for different use cases.
338-355: Added trigger registry systemImplements a robust system for registering and unregistering toggle triggers, managing event listeners, and handling accessibility attributes. This is a significant enhancement that supports the new orchestrator components mentioned in the PR.
357-366: Enhanced component registration with trigger supportExtends the global registry registration to include the new trigger management functions and component instance, improving the overall architecture and making these functions available throughout the application.
367-371: Accessibility improvements for trigger elementsImplements ARIA attributes and class toggling for trigger elements, making the components more accessible by providing proper state indication to assistive technologies.
373-377: Synchronized trigger state on model changesEnsures all registered triggers correctly reflect the current state whenever the model value changes, maintaining consistency between UI state and component state.
380-383: Improved cleanup of trigger event listenersProperly cleans up all registered trigger event listeners on component unmount, preventing memory leaks and ghost events.
429-434: Properly sequenced focus trap and event emissionUses requestAnimationFrame and nextTick to ensure the focus trap is activated and the 'shown' event is emitted in the correct sequence after animations complete.
435-439: Conditional promise resolution based on operation typeImplements the
_resolveOnHideflag to control when promises resolve - either immediately after the show animation or deferred until the component is hidden. This supports more complex use cases like confirmation dialogs.
445-445: Focus trap deactivation on leaveEnsures the focus trap is deactivated at the beginning of the leave transition, improving accessibility by releasing focus at the right moment.
453-453: Enhanced hidden event with trigger contextPasses the trigger information to the 'hidden' event, allowing consumers to determine what caused the component to hide, which is valuable for analytics and complex interaction flows.
466-468: Promise cleanup after hide completionProperly resolves and cleans up the promise and resolver after the hide animation completes, preventing memory leaks and ensuring the composable can be reused.
There was a problem hiding this comment.
@xvaara If you think this looks fine, please double check to review, then please squash merge with the correct conventional commits message with all of the breaking changes here. Other than that reservation, I think this looks good.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/docs/src/docs/composables/useToggle.md (1)
34-35: Add article "a" before "particular component" and fix compound sentence.Small grammatical improvements would make this paragraph clearer.
-You can also provide an id to get particular component and show/hide it. Currently, we do not support using CSS selector to -find modal since the component in lazy mode may not render at page initial. If the modal component does not exist and you attempt to call any of the exposed methods, the methods will safely no-op without throwing errors. +You can also provide an id to get a particular component and show/hide it. Currently, we do not support using CSS selector to +find modal since the component in lazy mode may not render at page initial. If the modal component does not exist and you attempt to call any of the exposed methods, the methods will safely no-op without throwing errors.🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...l. If the modal component does not exist and you attempt to call any of the exposed ...(COMMA_COMPOUND_SENTENCE)
🧹 Nitpick comments (4)
apps/docs/src/docs/composables/useToggle.md (4)
5-6: Add spacing after comma and fix grammatical issues.There's a missing space after the comma in "show,hide" and the sentence structure could be improved.
-You can use `useToggle` to get the closest toggleable component in **child component** and show,hide or toggle it. It can also be supplied a target id to show, hide or toggle a specific component +You can use `useToggle` to get the closest toggleable component in a **child component** and show, hide, or toggle it. It can also be supplied a target id to show, hide, or toggle a specific component.
69-69: Fix typo and clarify promise resolution behavior.There's a spelling error and the paragraph would benefit from more structured explanation.
-The `show` and `toggle` methods take a boolean argument to control wether to resolve the promise on show (`false`) or on hide (`true`). On `show` the promise resolves to `true` when shown and to `'show-prevented'` if show is prevented. On `hide` the promise resolves to the trigger that caused the hide event. The promise can be awaited to get the result. +The `show` and `toggle` methods take a boolean argument to control whether to resolve the promise on show (`false`) or on hide (`true`). The promise resolves as follows: +- When using `show()`: resolves to `true` when shown successfully or to `'show-prevented'` if show is prevented +- When using `hide()`: resolves to the trigger string that caused the hide event +- The promise can be awaited to get the result
84-95: Enhance the example to show both Promise patterns.The example demonstrates
.then()but notasync/await, which was mentioned as a possibility. Adding both patterns would be more educational.const toggle = useToggle('toggleTest') async function testToggle() { - toggle.show(true).then((e) => { - if (e === 'ok') { - console.log('ok pressed') - } else { - console.log('closed with', e) - } - }) + // Using Promise.then() pattern + toggle.show(true).then((result) => { + if (result === 'ok') { + console.log('OK button pressed') + } else { + console.log('Modal closed with reason:', result) + } + }) + + // Or using async/await pattern + // const result = await toggle.show(true) + // if (result === 'ok') { + // console.log('OK button pressed') + // } else { + // console.log('Modal closed with reason:', result) + // } }
118-130: Add error handling to the Promise implementation.The real-world implementation should include error handling for a robust async flow.
async function testToggle() { reason.value = '' toggle .show(true) .then((e) => { if (e === 'ok') { console.log('ok pressed') } else { console.log('closed with', e) } reason.value = e }) + .catch((error) => { + console.error('An error occurred:', error) + reason.value = 'error' + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/docs/src/docs/composables/useToggle.md(1 hunks)packages/bootstrap-vue-next/src/composables/useShowHide.ts(11 hunks)packages/bootstrap-vue-next/src/composables/useToggle/index.ts(1 hunks)packages/bootstrap-vue-next/src/utils/floatingUi.ts(2 hunks)packages/bootstrap-vue-next/src/utils/keys.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bootstrap-vue-next/src/composables/useToggle/index.ts
- packages/bootstrap-vue-next/src/utils/floatingUi.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/bootstrap-vue-next/src/utils/keys.ts (2)
packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts (12)
ToastOrchestratorCreateParam(144-144)PromiseWithToast(36-36)ControllerKey(12-12)ModalOrchestratorMapValue(211-225)ModalOrchestratorCreateParam(245-245)ModalOrchestratorCreateOptions(247-254)PromiseWithModal(23-25)PromiseWithModalBoolean(31-34)PopoverOrchestratorMapValue(178-192)PopoverOrchestratorCreateParam(209-209)PromiseWithPopover(47-49)TooltipOrchestratorCreateParam(176-176)packages/bootstrap-vue-next/src/types/index.ts (4)
ToastOrchestratorCreateParam(67-67)ModalOrchestratorCreateParam(69-69)PopoverOrchestratorCreateParam(68-68)TooltipOrchestratorCreateParam(66-66)
packages/bootstrap-vue-next/src/composables/useShowHide.ts (3)
packages/bootstrap-vue-next/src/composables/useModal/index.ts (2)
hide(53-55)show(50-52)packages/bootstrap-vue-next/src/utils/keys.ts (1)
globalShowHideStorageInjectionKey(175-176)packages/bootstrap-vue-next/src/utils/dom.ts (1)
isVisible(32-47)
🪛 LanguageTool
apps/docs/src/docs/composables/useToggle.md
[uncategorized] ~35-~35: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...l. If the modal component does not exist and you attempt to call any of the exposed ...
(COMMA_COMPOUND_SENTENCE)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/src/docs/composables/useToggle.md
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (18)
packages/bootstrap-vue-next/src/utils/keys.ts (7)
18-31: Updated composable type imports align with the new lifecycle management approach.These changes introduce new types like
PromiseWithModal,PromiseWithModalBoolean,PromiseWithPopover, andPromiseWithToastfor improved type safety in promise-based APIs. The naming convention change from "ShowParam" to "CreateParam" reflects the new composable API pattern discussed in the PR objectives.
148-157: Good refactoring of the show/hide component system to use promises for state management.The
RegisterShowHideFnInputinterface has been enhanced to:
- Include the component instance for better lifecycle tracking
- Use promise-based methods for toggle/show/hide operations
- Add trigger registration methods for more flexibility
This change aligns with the PR objectives of improving composable APIs and lifecycle management.
159-168: Well-structured interface for storing show/hide component state.The new
RegisterShowHideMapValueinterface properly captures the state and behavior needed for the show/hide registry. The promise return types includenullto handle edge cases, and the additionalnoTraverseparameter inhide()provides more control over the hiding behavior.
169-174: Improved registry with Map-based storage for better performance and type safety.The change from using a plain object to a reactive Map for storing show/hide component references offers better performance for lookups and removes string-based key limitations. This is a good practice for managing a dynamic collection of components.
220-230: Updated toast plugin with new API pattern while maintaining backward compatibility.The toast plugin now includes:
- An orchestrator installation flag to detect if the required component is mounted
- A new promise-based
create()method- Properly deprecated
show()methodThis implementation aligns with the discussions in the PR about handling composable API design and lifecycle management.
232-249: Modal controller refactoring with improved lifecycle management.The updated modal controller:
- Now uses a Map for better component instance management
- Exposes an orchestrator installation status flag
- Introduces a more flexible
create()method with options for controlling the modal lifecycle- Properly deprecates older methods while maintaining backward compatibility
This implementation addresses the discussions around modal lifecycle management mentioned in the PR objectives, particularly regarding whether modals should be kept alive or disposed.
251-256: Unified popover/tooltip plugin with promise-based API.The popover plugin now:
- Uses a Map for tracking component instances by key
- Exposes distinct methods for popover and tooltip creation
- Includes an orchestrator installation status flag
This change aligns with the overall composable API pattern being established across other components.
packages/bootstrap-vue-next/src/composables/useShowHide.ts (11)
178-180: New promise management variables added.These variables enable the new promise-based API, allowing asynchronous control of show/hide operations. This is a good architectural improvement that aligns with modern async patterns.
181-189: The show method now returns a Promise for better async flow control.The method now supports early returns for efficiency and maintains proper promise state. This is a significant improvement that enables awaitable show operations and better error handling.
246-255: The hide method now returns a Promise and tracks trigger context.Excellent enhancement to track which trigger caused the hide action. This contextual information improves debugging and enables more sophisticated UI interactions.
285-285: Focus trap deactivation moved to beginning of hide process.Good accessibility improvement to immediately release the focus trap when hiding starts, rather than waiting for transitions to complete.
315-326: The toggle method now returns a Promise with proper resolution.This change brings toggle in line with the other Promise-based methods and provides a clean API for async toggle operations.
340-357: New trigger registry system for improved accessibility.This implementation properly manages ARIA attributes and CSS classes for trigger elements. Great for accessibility compliance and visual state feedback.
359-368: Enhanced global registry integration.Now includes the component instance and trigger management functions in the registry. This improves the coordination between different parts of the UI system.
369-379: Added visibility state management for triggers.This ensures that trigger elements correctly reflect the current state of their target components through both ARIA attributes and CSS classes.
431-442: Promise resolution logic in onAfterEnter transition hook.The conditional resolution based on the resolveOnHide parameter provides flexibility in how promises are handled. This supports different use cases for modal interaction patterns.
455-455: Enhanced hidden event with trigger information.Including the trigger in the hidden event provides valuable context about why the component was hidden, enabling more sophisticated event handling.
468-471: Clean promise resolution and state cleanup.Good practice to resolve the promise with the trigger context and properly clean up state variables to prevent memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/playground/src/components/Comps/TCarousel.vue (2)
64-78: Dynamic carousel implementation demonstrates reactive slide renderingThis new carousel example effectively demonstrates how to dynamically render slides using a reactive array. Consider adding the same configuration options as other examples (controls, indicators, interval) for consistency.
- <BCarousel> + <BCarousel controls indicators :interval="5000" ride="carousel">
68-70: Using index as key may cause issues with dynamic updatesWhile using the array index as a key works for this example, it could cause unexpected behavior if items are reordered or removed in a real application.
- v-for="(image, index) in images" - :key="index" + v-for="(image, index) in images" + :key="image + '-' + index"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/playground/src/components/Comps/TCarousel.vue(1 hunks)packages/bootstrap-vue-next/src/components/BCarousel/BCarousel.vue(1 hunks)
🔇 Additional comments (1)
packages/bootstrap-vue-next/src/components/BCarousel/BCarousel.vue (1)
24-24: Improved robustness with optional chainingAdding optional chaining (
?.[i]?._id) to thearia-describedbybinding prevents potential runtime errors whenslideValuesor a specific index might be undefined or null during component initialization or dynamic updates.
|
Lmk when you're done |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (2)
49-65:this‐bound methods are still fragile when the object is destructured
show,hide, andtoggleare declared with the method syntax and rely onthis.ref.
If a consumer later extracts the methods (const {show} = toast; show()),thisbecomesundefined, throwing at runtime.The previous review highlighted this hazard, but the implementation remains unchanged.
135-137:⚠️ Potential issue
id≠_self: public-type mismatch breaks type-safety
_selfis the documented identifier inToastOrchestratorParam, yet the code still readsresolvedProps.value?.id.
Callers following the public API cannot supplyid, so_selfalways falls back to a newSymbol, making it impossible to interact with the toast by a predictable key.-const _self = resolvedProps.value?.id || Symbol('Toast controller') +const _self = + (resolvedProps.value?._self as ControllerKey | undefined) ?? + Symbol('Toast controller')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts(1 hunks)packages/bootstrap-vue-next/src/plugins/toastController/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bootstrap-vue-next/src/plugins/modalController/index.ts
🧰 Additional context used
🧠 Learnings (1)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (3)
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
packages/bootstrap-vue-next/src/plugins/toastController/index.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (2)
49-63: 🛠️ Refactor suggestionAvoid context loss on destructured methods
The
show,hide, andtogglemethods aren't arrow functions, so they'll losethiscontext if destructured. This could cause subtle bugs if users extract these methods without binding.- show() { + show: () => { if (!this.ref) return this.set({modelValue: true}) this.ref.show() return promise }, - hide(trigger?: string) { + hide: (trigger?: string) => { if (!this.ref) return this.set({modelValue: false}) this.ref.hide(trigger, true) return promise }, - toggle() { + toggle: () => { if (!this.ref) return this.set({modelValue: !this.get()?.modelValue}) this.ref.toggle() return promise },
150-163: 🛠️ Refactor suggestionWatcher mutates the observed object
In the watch callback,
newValue.props = undefinedmutates the observed object. This could potentially create an infinite watch loop or unexpected behavior.Consider creating a new object instead of modifying the one being watched:
- if (newValue.props) { - Object.assign(v, newValue.props) - newValue.props = undefined - } + if (newValue.props) { + Object.assign(v, newValue.props) + // Create a new object instead of mutating newValue + newValue = { ...newValue, props: undefined } + }
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (1)
200-219: Well-handled deprecation of show/confirm methodsThe deprecated methods include clear console warnings that guide users toward the new API pattern. The implementation preserves backward compatibility while encouraging migration to the more flexible
create().show()pattern.Consider adding a target version for removal in the deprecation message to help users plan their migration timeline.
- '[BootstrapVueNext] useModalController: The show() method is deprecated. Use create({}).show() instead.' + '[BootstrapVueNext] useModalController: The show() method is deprecated and will be removed in v1.0. Use create({}).show() instead.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (3)
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
🔇 Additional comments (7)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (7)
32-33: Great addition of the orchestrator installation checkAdding this flag helps prevent the common error of trying to use modals before the orchestrator component is mounted, which would cause silent failures. The error message in lines 120-124 clearly communicates the issue to developers.
89-98: Well-implemented Promise with cleanupThe Promise handling during modal destruction is properly implemented without using async in the constructor, which avoids unhandled rejections. The approach with nextTick and waiting for the 'hidden' event ensures the modal is completely removed from the DOM before resolving.
102-104: Excellent use of Symbol.asyncDisposeImplementing the Symbol.asyncDispose symbol allows for automatic cleanup when used with the JavaScript "using" statement, which is a modern best practice for resource management. This enables users to write code like:
using modal = await useModalController().create({title: 'My Modal'}); modal.show(); // Modal will be automatically cleaned up when the scope exits
126-134: Proper handling of components and slotsUsing
markRawon components and slots is crucial to prevent Vue from making them reactive, which would cause errors or performance issues. This is a good defensive programming practice.
167-181: Good reactive handling of modelValue updatesThe implementation properly handles all scenarios for modelValue updates:
- Calls the user's update handler
- Updates the ref if the input was a ref
- Updates the modelValue directly if it was passed as a ref
- Updates the internal modals map
This comprehensive approach ensures reactivity works correctly in all scenarios.
191-197: Proper use of onScopeDispose with failSilently parameterThe
onScopeDisposefunction correctly uses the second parametertrueto make it fail silently when called outside a component setup context. This is a good use of Vue 3.5+'s enhanced API.
221-227: Complete and consistent provider APIThe provider API is complete, exposing both new and deprecated methods through the modal controller plugin key. This gives developers flexibility during migration.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (2)
152-159:⚠️ Potential issueMutating watched object in watcher causes potential infinite loop
Setting
newValue.props = undefinedandnewValue.pos = undefinedmutates the watched source object. Since the watcher is deep, this can trigger another update cycle.Work with a clone instead of mutating the original object:
-if (newValue.props) { - Object.assign(v, newValue.props) - newValue.props = undefined -} -if (newValue.pos) { - v.position = newValue.pos - newValue.pos = undefined -} +if (newValue.props) { + Object.assign(v, newValue.props) +} +if (newValue.pos) { + v.position = newValue.pos +}
51-65:⚠️ Potential issueMethods will lose
thiscontext when destructuredThe
show(),hide(), andtoggle()methods rely onthis.refandthis.get()which will break if destructured (const { show } = toast; show()). This leads to runtime errors when developers use common destructuring patterns.Consider using arrow functions that capture variables instead of relying on
this:-show() { - if (!this.ref) return this.set({modelValue: true}) - this.ref.show() - return promise -}, +show: () => { + if (!promise.ref) return promise.set({modelValue: true}) + promise.ref.show() + return promise +},packages/bootstrap-vue-next/src/plugins/modalController/index.ts (2)
49-66:⚠️ Potential issueMethods will lose
thiscontext when destructuredThe
show(),hide(), andtoggle()methods rely onthis.refandthis.get()which will break if destructured. This is the same issue found in the toast controller.Consider using arrow functions that capture variables instead of relying on
this:-show() { - if (!this.ref) return this.set({modelValue: true}) - this.ref.show() - return promise -}, +show: () => { + if (!promise.ref) return promise.set({modelValue: true}) + promise.ref.show() + return promise +},
152-155:⚠️ Potential issueMutating watched object in watcher causes potential infinite loop
Setting
newValue.props = undefinedinside the watcher mutates the watched source object, potentially triggering additional update cycles.Work with a clone instead of mutating the original object:
-if (newValue.props) { - Object.assign(v, newValue.props) - newValue.props = undefined -} +if (newValue.props) { + Object.assign(v, newValue.props) +}packages/bootstrap-vue-next/src/plugins/popoverController/index.ts (2)
52-66:⚠️ Potential issueMethods will lose
thiscontext when destructuredThe
show(),hide(), andtoggle()methods rely onthis.refandthis.get()which will break if destructured. This is the same issue found in the other controllers.Consider using arrow functions that capture variables instead of relying on
this:-show() { - if (!this.ref) return this.set({modelValue: true}) - this.ref.show() - return promise -}, +show: () => { + if (!promise.ref) return promise.set({modelValue: true}) + promise.ref.show() + return promise +},
154-163: 🛠️ Refactor suggestionPotential modification of watched objects in the watcher
While this code doesn't explicitly set properties to undefined like the other controllers, it's still manipulating properties directly on the watched object, which could lead to unwanted recursion.
Consider working with a cloned object to avoid side effects on the watched source:
-for (const key in newValue) { - if (key.startsWith('on')) { - v[key as keyof PopoverOrchestratorCreateParam] = - newValue[key as keyof PopoverOrchestratorCreateParam] - } else { - v[key as keyof PopoverOrchestratorCreateParam] = toValue( - newValue[key as keyof PopoverOrchestratorCreateParam] - ) - } -} +// Create a clone of newValue to avoid mutating the watched object +const clonedValue = {...newValue}; +for (const key in clonedValue) { + if (key.startsWith('on')) { + v[key as keyof PopoverOrchestratorCreateParam] = + clonedValue[key as keyof PopoverOrchestratorCreateParam] + } else { + v[key as keyof PopoverOrchestratorCreateParam] = toValue( + clonedValue[key as keyof PopoverOrchestratorCreateParam] + ) + } +}
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (1)
119-125: JSDoc needs to be updated to fully describe all parametersThe JSDoc comment describes the return value but doesn't document the
optionsparameter.Update the JSDoc to include all parameters:
/** + * @param {ToastOrchestratorCreateParam} obj - The toast configuration object + * @param {ToastOrchestratorCreateOptions} options - Additional options for toast creation * @returns {PromiseWithToast} Returns a promise object with methods to control the toast (show, hide, toggle, get, set, destroy) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts(1 hunks)packages/bootstrap-vue-next/src/plugins/popoverController/index.ts(1 hunks)packages/bootstrap-vue-next/src/plugins/toastController/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (3)
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (3)
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/toastController/index.ts:203-208
Timestamp: 2025-04-22T20:13:41.342Z
Learning: In Vue 3.5+, the `onScopeDispose` function accepts two parameters: `function onScopeDispose(fn: () => void, failSilently?: boolean): void`. The second parameter `failSilently` controls whether to throw an error or silently ignore if the function is called outside of a component setup context.
packages/bootstrap-vue-next/src/plugins/popoverController/index.ts (1)
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/popoverController/index.ts:138-140
Timestamp: 2025-04-28T22:48:46.712Z
Learning: In bootstrap-vue-next, `PopoverOrchestratorParam` includes an `id` field of type `ControllerKey` through inheritance from base types. This field is declared in the `BvControllerOptions` interface and propagated through the type hierarchy.
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/plugins/popoverController/index.ts (4)
packages/bootstrap-vue-next/src/plugins/index.ts (1)
popoverPlugin(6-6)packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts (9)
ControllerKey(12-12)PopoverOrchestratorMapValue(178-192)TooltipOrchestratorMapValue(146-159)PromiseWithPopover(47-49)PopoverOrchestratorParam(194-207)TooltipOrchestratorParam(161-174)PromiseWithPopoverInternal(50-59)PopoverOrchestratorCreateParam(209-209)TooltipOrchestratorCreateParam(176-176)packages/bootstrap-vue-next/src/utils/classes.ts (1)
BvTriggerableEvent(68-90)packages/bootstrap-vue-next/src/utils/keys.ts (1)
popoverPluginKey(251-256)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (15)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (5)
86-110: Thedestroy()method properly preserves event handlersThe implementation correctly preserves any existing
onHiddenhandlers when adding the promise resolution handler, avoiding issues with overwriting user-supplied callbacks.
211-212: Return value properly implements the Promise with toast controllerThe
createfunction correctly returns the enhanced promise object, allowing users to both await the promise and use the fluent controller API.
205-210: Proper scope disposal for cleaning up toastsThe
onScopeDisposehandler correctly uses the second parametertrueto silently ignore errors when called outside component setup. This was a point of discussion in previous reviews and is correctly implemented here.
214-223: Appropriate deprecation messaging for the legacy methodThe deprecated
showmethod includes a clear console warning to guide users toward the new API, while maintaining backward compatibility.
137-139:❌ Incorrect review comment
Type inconsistency between
idand_selfThe code extracts
idfromresolvedProps.valuebut identifies toasts using_selfinternally, which can lead to type confusion.Consider aligning the identifier names:
-const _self = resolvedProps.value?.id || Symbol('Toast controller') +const _self = resolvedProps.value?._self || Symbol('Toast controller')To verify this issue, check if the types match up:
🏁 Script executed:
#!/bin/bash # Check if ToastOrchestratorParam includes an id property grep -A 20 "export type ToastOrchestratorParam" packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.tsLength of output: 768
🏁 Script executed:
#!/bin/bash # Locate the BToastProps type definition grep -R "export .*BToastProps" -n packages/bootstrap-vue-next/src # Pick the first result and inspect occurrences of 'id' in that file FILE=$(grep -R "export .*BToastProps" -n packages/bootstrap-vue-next/src | head -n1 | cut -d: -f1) echo "Inspecting 'id' in $FILE" grep -n "id" "$FILE"Length of output: 2845
🏁 Script executed:
#!/bin/bash # Locate resolvedProps usages and print context around the snippet FILE="packages/bootstrap-vue-next/src/plugins/toastController/index.ts" grep -n "resolvedProps" "$FILE" echo "---- Context around lines 130-150 ----" sed -n '130,150p' "$FILE"Length of output: 1353
Ignore this suggestion:
idvs._selfare intentionally distinct
Theidprop onresolvedProps.valueis a user-supplied ControllerKey (string or symbol), while_selfis an internal identifier for the orchestrator (falling back to a new Symbol when noidis provided). They share the sameControllerKeytype, and renamingresolvedProps.value.idtoresolvedProps.value._selfwould be invalid (props don’t include_self). No change needed here.Likely an incorrect or invalid review comment.
packages/bootstrap-vue-next/src/plugins/modalController/index.ts (4)
122-126: Proper validation of orchestrator installationThe check for
_isOrchestratorInstalledwith a helpful error message is a good practice for ensuring the plugin is properly configured before use.
84-107: Good pattern for cleaning up modal stateThe
destroymethod properly manages the modal lifecycle - stopping the watcher, ensuring the modal is hidden, preserving event handlers, and finally removing it from the storage map.
193-199: Proper scope disposal for cleaning up modalsThe
onScopeDisposehandler correctly uses the second parametertrueto handle being called outside a component scope.
205-221: Clear deprecation approach for legacy methodsThe deprecated
showandconfirmmethods include appropriate warnings while providing backward compatibility. This supports a smooth migration path to the new API.packages/bootstrap-vue-next/src/plugins/popoverController/index.ts (6)
31-34: Using a unified Map for both popovers and tooltipsThe refactoring to use a single Map to store both popovers and tooltips simplifies the codebase and encourages code reuse. This is a good architectural decision.
91-103: Promise in destroy method correctly handles event handlersThe implementation properly preserves existing event handlers by storing the previous handler and chaining them, which prevents overwriting user callbacks.
125-129: Proper validation of orchestrator installationThe check for
_isOrchestratorInstalledwith a descriptive error message ensures developers have properly configured the plugin.
201-203: Clean abstraction for different component typesCreating helper functions
tooltip()andpopover()that wrap thecreate()method with the appropriate flag provides a clean abstraction for users.
191-196: Proper scope disposal for cleaning up popoversThe
onScopeDisposehandler correctly uses the second parametertrueto handle being called outside a component scope, consistent with the other controllers.
140-141:❌ Incorrect review comment
Accessing
idproperty that may not be in the type definitionBased on previous discussions, there may be a discrepancy between the code accessing
resolvedProps.value?.idand what's declared in the type definition.
🏁 Script executed:
#!/bin/bash # Verify if the PopoverOrchestratorParam type includes an id field grep -A 10 "export type PopoverOrchestratorParam" packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts grep -A 10 "export interface BvControllerOptions" packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts || echo "BvControllerOptions not found"Length of output: 792
Let’s verify how the
idfield gets injected by the orchestrator’s base types:
🏁 Script executed:
#!/bin/bash # Search for BvControllerOptions in the entire repo rg "BvControllerOptions" -n . # Locate the ControllerKey type rg "type ControllerKey" -n packages/bootstrap-vue-next/src # Find the orchestrator setup to see what props are mixed in rg "createOrchestrator" -n packages/bootstrap-vue-next/src rg "createController" -n packages/bootstrap-vue-next/srcLength of output: 321
No output was captured—let's refine our approach to locate the file. It appears the path may differ. Let’s list the directory contents under
packages/bootstrap-vue-next/src/pluginsto confirm the exact path.
🏁 Script executed:
#!/bin/bash ls -R packages/bootstrap-vue-next/src/pluginsLength of output: 782
Let’s inspect how
createControlleris imported and whatresolvedPropsis typed as:
🏁 Script executed:
#!/bin/bash # Find the import of createController in popoverController rg "createController" -n packages/bootstrap-vue-next/src/plugins/popoverController/index.ts # Dump the first 200 lines to see how resolvedProps is defined and used sed -n '1,200p' packages/bootstrap-vue-next/src/plugins/popoverController/index.tsLength of output: 6436
Let’s check if
idis defined on the create-param or map-value types:
🏁 Script executed:
#!/bin/bash # Show PopoverOrchestratorCreateParam definition grep -R "export type PopoverOrchestratorCreateParam" -n packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts -A10 # Show PopoverOrchestratorMapValue definition grep -R "export type PopoverOrchestratorMapValue" -n packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.ts -A10Length of output: 1178
To confirm whether
idcomes fromBPopoverProps, let’s inspect the top of the orchestrator types file:
🏁 Script executed:
#!/bin/bash sed -n '1,100p' packages/bootstrap-vue-next/src/types/ComponentOrchestratorTypes.tsLength of output: 3813
To confirm whether
idcomes fromBPopoverProps, let’s inspect its definition:
🏁 Script executed:
#!/bin/bash # Show BPopoverProps to see if `id` is declared grep -R "export interface BPopoverProps" -n packages/bootstrap-vue-next/src/types/ComponentProps.ts -A20Length of output: 700
No issue with
idaccess:idis defined inBPopoverProps
ThePopoverOrchestratorParamtype is built asOmit<BPopoverProps, 'body' | 'title' | 'modelValue'> & …, andBPopoverPropsdeclaresid?: string. AccessingresolvedProps.value?.idis therefore correct and needs no change.Likely an incorrect or invalid review comment.
* upstream/main: chore: release main (bootstrap-vue-next#2690) docs(BProgress): Parity pass (bootstrap-vue-next#2689) fix(BTableSimple): fixed and nobordercollapse to work fixes bootstrap-vue-next#2685 docs: fix incorrect references and missed script sections (bootstrap-vue-next#2686) docs: implement on this page expand/collapse with useScrollspy (bootstrap-vue-next#2679) chore: release main (bootstrap-vue-next#2683) feat(BTable): implement 'fixed' and 'noBorderCollapse' props (bootstrap-vue-next#2681) chore: release main (bootstrap-vue-next#2678) Update package.json fix(BFormSelect): prevent options with label from being treated as groups (bootstrap-vue-next#2666) fix: patch regression issue in bootstrap-vue-next#2665 (bootstrap-vue-next#2670) Update release-main.yaml chore: release main (bootstrap-vue-next#2660) chore: update depencies fix(BTabs): corrent classes on ssr (bootstrap-vue-next#2664) Changes to public composables (bootstrap-vue-next#2425) docs(BTable): parity pass (bootstrap-vue-next#2669)
feat(BAlert)!: make act like toast, useShowHide. feat(useShowHide): create triggerRegistry for adding external triggers (like in vBToggle) fix: type popoverController fix(useShowHide): focustrap off at the begining of leave, pass down the trigger to other hide emits. fix(vBToggle): keep track of targets fix(BPopover)!: change prop content to body to align with other components fix(BTooltip)!: change prop content to body to align with other components feat(usePopoverController): allow more options fix(vBToggle): find late components, ie. inside ClientOnly fix(useModalController)!: move props to main level, add slots feat(usePopoverController): add slots feat(useToastController)!: remove props obj, the parameters are flat now. Add slots, rename pos -> position feat(useShowHide): show returns a promise, resolve on show or hide. feat(useToggle): toggle any show/hide component feat!: controller composables functions return promise, with chainable functions feat(useModalController): add support for using syntax in ts feat(BModal): add okClass and cancelClass to add classes to the buttons. feat(useModalController)!: change of api, check the docs fix: inline functional style to show toast,modal and dropdown feat(useToggle): add trigger to promise resolve on hide. fix(BCarousel): fix v-for updates
commit 2a9e30b Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Thu May 15 18:24:07 2025 +0300 doc data commit 08c89fd Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Thu May 15 17:57:29 2025 +0300 feat(BPopover): add titleClass and bodyClass, remove unneeded customClass prop since class is inherited to the same place commit 90b578d Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Wed May 14 11:39:42 2025 +0300 feat(BToast): add noProgress prop, make progress show as default if modelValue is number. fix(useToastController): if using the deprecated show method the countdown didn't start. commit dc85d94 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun May 11 09:53:25 2025 -0500 chore: release main (bootstrap-vue-next#2690) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 070cb8c Author: David W. Gray <dwgray67@hotmail.com> Date: Sun May 11 07:52:30 2025 -0700 docs(BProgress): Parity pass (bootstrap-vue-next#2689) commit c61f532 Author: Thierry Blind <tbl0605@gmail.com> Date: Sun May 11 16:52:14 2025 +0200 fix(BTableSimple): fixed and nobordercollapse to work fixes bootstrap-vue-next#2685 commit beae36f Author: David W. Gray <dwgray67@hotmail.com> Date: Sun May 11 07:43:58 2025 -0700 docs: fix incorrect references and missed script sections (bootstrap-vue-next#2686) commit 34432d9 Author: David W. Gray <dwgray67@hotmail.com> Date: Sun May 11 07:42:02 2025 -0700 docs: implement on this page expand/collapse with useScrollspy (bootstrap-vue-next#2679) commit ce869f0 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed May 7 11:16:08 2025 -0500 chore: release main (bootstrap-vue-next#2683) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 9486276 Author: Mohamed Nasri <51300752+mhn147@users.noreply.github.com> Date: Wed May 7 09:44:38 2025 -0600 feat(BTable): implement 'fixed' and 'noBorderCollapse' props (bootstrap-vue-next#2681) commit a4a9294 Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon May 5 09:16:09 2025 -0500 chore: release main (bootstrap-vue-next#2678) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 0165e94 Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Mon May 5 16:24:04 2025 +0300 Update package.json commit c1645a9 Author: Rajitha <rajithaeye@gmail.com> Date: Wed Apr 30 23:49:23 2025 +0530 fix(BFormSelect): prevent options with label from being treated as groups (bootstrap-vue-next#2666) commit 59ddc39 Author: Thierry Blind <tbl0605@gmail.com> Date: Wed Apr 30 20:17:34 2025 +0200 fix: patch regression issue in bootstrap-vue-next#2665 (bootstrap-vue-next#2670) commit d82091b Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Wed Apr 30 06:01:10 2025 +0300 Update release-main.yaml commit 31cb4bf Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed Apr 30 04:51:31 2025 +0300 chore: release main (bootstrap-vue-next#2660) Co-Authored-By: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> commit 6733770 Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Wed Apr 30 05:15:02 2025 +0300 chore: update depencies commit 2b37c18 Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Wed Apr 30 04:36:46 2025 +0300 fix(BTabs): corrent classes on ssr (bootstrap-vue-next#2664) fix(BTabs): corrent classes on ssr fix(BTabs): fix another recursion error commit 99718eb Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Wed Apr 30 04:20:00 2025 +0300 Changes to public composables (bootstrap-vue-next#2425) feat(BAlert)!: make act like toast, useShowHide. feat(useShowHide): create triggerRegistry for adding external triggers (like in vBToggle) fix: type popoverController fix(useShowHide): focustrap off at the begining of leave, pass down the trigger to other hide emits. fix(vBToggle): keep track of targets fix(BPopover)!: change prop content to body to align with other components fix(BTooltip)!: change prop content to body to align with other components feat(usePopoverController): allow more options fix(vBToggle): find late components, ie. inside ClientOnly fix(useModalController)!: move props to main level, add slots feat(usePopoverController): add slots feat(useToastController)!: remove props obj, the parameters are flat now. Add slots, rename pos -> position feat(useShowHide): show returns a promise, resolve on show or hide. feat(useToggle): toggle any show/hide component feat!: controller composables functions return promise, with chainable functions feat(useModalController): add support for using syntax in ts feat(BModal): add okClass and cancelClass to add classes to the buttons. feat(useModalController)!: change of api, check the docs fix: inline functional style to show toast,modal and dropdown feat(useToggle): add trigger to promise resolve on hide. fix(BCarousel): fix v-for updates commit 340edfd Author: David W. Gray <dwgray67@hotmail.com> Date: Mon Apr 28 18:39:44 2025 -0700 docs(BTable): parity pass (bootstrap-vue-next#2669) commit 4dd6c89 Author: Jukka Raimovaara <roska@mentalhouse.fi> Date: Mon Apr 28 22:46:31 2025 +0300 fix(BDropdown): don't calulcate the position when dropdown is not shown.
BEGIN_COMMIT_OVERRIDE
feat(BAlert)!: make act like toast, useShowHide.
feat(useShowHide): create triggerRegistry for adding external triggers (like in vBToggle)
fix: type popoverController
fix(useShowHide): focustrap off at the begining of leave, pass down the trigger to other hide emits.
fix(vBToggle): keep track of targets
fix(BPopover)!: change prop content to body to align with other components
fix(BTooltip)!: change prop content to body to align with other components
feat(usePopoverController): allow more options
fix(vBToggle): find late components, ie. inside ClientOnly
fix(useModalController)!: move props to main level, add slots
feat(usePopoverController): add slots
feat(useToastController)!: remove props obj, the parameters are flat now. Add slots, rename pos -> position
feat(useShowHide): show returns a promise, resolve on show or hide.
feat(useToggle): toggle any show/hide component
feat!: controller composables functions return promise, with id and chainable functions
feat(useModalController): add support for using syntax in ts
feat(BModal): add okClass and cancelClass to add classes to the buttons.
feat(useModalController)!: change of api, check the docs
fix: inline functional style to show toast,modal and dropdown
feat(useToggle): add trigger to promise resolve on hide.
fix(BCarousel): fix v-for updates
END_COMMIT_OVERRIDE
Describe the PR
Lot's of changes to composables.
Small replication
A small replication or video walkthrough can help demonstrate the changes made. This is optional, but can help observe the intended changes. A mentioned issue that contains a replication also works.
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)feat(...)fix(...)docs(...)The PR fulfills these requirements:
CHANGELOGis generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be deniedSummary by CodeRabbit
useTogglecomposable with documentation and Table of Contents integration.createmethods and richer lifecycle management.bodyprop consistently, replacing deprecatedcontent.