Reviewing files that changed from the base of the PR and between 5a77dcb and a6d73e4.

• 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)

• packages/bootstrap-vue-next/src/composables/useToggle/index.ts
• packages/bootstrap-vue-next/src/utils/floatingUi.ts

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, and PromiseWithToast for 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 RegisterShowHideFnInput interface has been enhanced to:

1. Include the component instance for better lifecycle tracking
2. Use promise-based methods for toggle/show/hide operations
3. 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 RegisterShowHideMapValue interface properly captures the state and behavior needed for the show/hide registry. The promise return types include null to handle edge cases, and the additional noTraverse parameter in hide() 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:

1. An orchestrator installation flag to detect if the required component is mounted
2. A new promise-based create() method
3. Properly deprecated show() method

This 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:

1. Now uses a Map for better component instance management
2. Exposes an orchestrator installation status flag
3. Introduces a more flexible create() method with options for controlling the modal lifecycle
4. 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:

1. Uses a Map for tracking component instances by key
2. Exposes distinct methods for popover and tooltip creation
3. 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.

Reviewing files that changed from the base of the PR and between a6d73e4 and fa1bd87.

• apps/playground/src/components/Comps/TCarousel.vue (1 hunks)
• packages/bootstrap-vue-next/src/components/BCarousel/BCarousel.vue (1 hunks)

packages/bootstrap-vue-next/src/components/BCarousel/BCarousel.vue (1)

24-24: Improved robustness with optional chaining

Adding optional chaining (?.[i]?._id) to the aria-describedby binding prevents potential runtime errors when slideValues or a specific index might be undefined or null during component initialization or dynamic updates.

Reviewing files that changed from the base of the PR and between fa1bd87 and e76a1f4.

• packages/bootstrap-vue-next/src/plugins/modalController/index.ts (1 hunks)
• packages/bootstrap-vue-next/src/plugins/toastController/index.ts (2 hunks)

• packages/bootstrap-vue-next/src/plugins/modalController/index.ts

Reviewing files that changed from the base of the PR and between e76a1f4 and d7157fa.

• packages/bootstrap-vue-next/src/plugins/modalController/index.ts (1 hunks)

packages/bootstrap-vue-next/src/plugins/modalController/index.ts (7)

32-33: Great addition of the orchestrator installation check

Adding 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 cleanup

The 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.asyncDispose

Implementing 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 slots

Using markRaw on 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 updates

The implementation properly handles all scenarios for modelValue updates:

1. Calls the user's update handler
2. Updates the ref if the input was a ref
3. Updates the modelValue directly if it was passed as a ref
4. Updates the internal modals map

This comprehensive approach ensures reactivity works correctly in all scenarios.

---

191-197: Proper use of onScopeDispose with failSilently parameter

The onScopeDispose function correctly uses the second parameter true to 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 API

The provider API is complete, exposing both new and deprecated methods through the modal controller plugin key. This gives developers flexibility during migration.

Reviewing files that changed from the base of the PR and between d7157fa and f678981.

• 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)

• GitHub Check: build

packages/bootstrap-vue-next/src/plugins/toastController/index.ts (5)

86-110: The destroy() method properly preserves event handlers

The implementation correctly preserves any existing onHidden handlers when adding the promise resolution handler, avoiding issues with overwriting user-supplied callbacks.

---

211-212: Return value properly implements the Promise with toast controller

The create function 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 toasts

The onScopeDispose handler correctly uses the second parameter true to 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 method

The deprecated show method includes a clear console warning to guide users toward the new API, while maintaining backward compatibility.

---

137-139:

❌ Incorrect review comment

Type inconsistency between id and _self

The code extracts id from resolvedProps.value but identifies toasts using _self internally, 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.ts

Length 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: id vs. _self are intentionally distinct
The id prop on resolvedProps.value is a user-supplied ControllerKey (string or symbol), while _self is an internal identifier for the orchestrator (falling back to a new Symbol when no id is provided). They share the same ControllerKey type, and renaming resolvedProps.value.id to resolvedProps.value._self would 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 installation

The check for _isOrchestratorInstalled with 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 state

The destroy method 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 modals

The onScopeDispose handler correctly uses the second parameter true to handle being called outside a component scope.

---

205-221: Clear deprecation approach for legacy methods

The deprecated show and confirm methods 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 tooltips

The 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 handlers

The 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 installation

The check for _isOrchestratorInstalled with a descriptive error message ensures developers have properly configured the plugin.

---

201-203: Clean abstraction for different component types

Creating helper functions tooltip() and popover() that wrap the create() method with the appropriate flag provides a clean abstraction for users.

---

191-196: Proper scope disposal for cleaning up popovers

The onScopeDispose handler correctly uses the second parameter true to handle being called outside a component scope, consistent with the other controllers.

---

140-141:

❌ Incorrect review comment

Accessing id property that may not be in the type definition

Based on previous discussions, there may be a discrepancy between the code accessing resolvedProps.value?.id and 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 id field 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/src

Length 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/plugins to confirm the exact path.

---

🏁 Script executed:

#!/bin/bash
ls -R packages/bootstrap-vue-next/src/plugins

Length of output: 782

---

Let’s inspect how createController is imported and what resolvedProps is 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.ts

Length of output: 6436

---

Let’s check if id is 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 -A10

Length of output: 1178

---

To confirm whether id comes from BPopoverProps, 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.ts

Length of output: 3813

---

To confirm whether id comes from BPopoverProps, 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 -A20

Length of output: 700

---

No issue with id access: id is defined in BPopoverProps
The PopoverOrchestratorParam type is built as Omit<BPopoverProps, 'body' | 'title' | 'modelValue'> & …, and BPopoverProps declares id?: string. Accessing resolvedProps.value?.id is therefore correct and needs no change.

Likely an incorrect or invalid review comment.