8000 draft of BApp by xvaara · Pull Request #2732 · bootstrap-vue-next/bootstrap-vue-next · GitHub
[go: up one dir, main page]

Skip to content

draft of BApp #2732

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/docs/src/docs/demo/ModalConfirm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
</template>

<script setup lang="ts">
import {useModalController} from 'bootstrap-vue-next'
import {type BvTriggerableEvent, useModalController} from 'bootstrap-vue-next'
import {ref} from 'vue'

const {confirm} = useModalController()
const confirmResult = ref<boolean | null | undefined>(null)
const confirmResult = ref<boolean | null | BvTriggerableEvent>(null)

const confirmBox = async () => {
confirmResult.value = await confirm?.({
Expand Down
4 changes: 2 additions & 2 deletions apps/docs/src/docs/demo/ModalMessageBox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
</template>

<script setup lang="ts">
import {useModalController} from 'bootstrap-vue-next'
import {type BvTriggerableEvent, useModalController} from 'bootstrap-vue-next'
import {ref} from 'vue'

const {show} = useModalController()

const okResult = ref<boolean | null | undefined>(undefined)
const okResult = ref<boolean | null | BvTriggerableEvent>(null)

const okBox = async () => {
okResult.value = await show?.({
Expand Down
14 changes: 8 additions & 6 deletions packages/bootstrap-vue-next/src/App.vue
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
<template>
<BContainer>
<BRow>
<BCol> Hello World! </BCol>
</BRow>
</BContainer>
<BApp>
<BContainer>
<BRow>
<BCol> Hello World! </BCol>
</BRow>
</BContainer>
</BApp>
</template>

<script setup lang="ts">
// You can use this file as a development spot to test your changes
// Please do not commit this file
import {BCol, BContainer, BRow} from './components'
import {BApp, BCol, BContainer, BRow} from './components'
</script>
263 changes: 263 additions & 0 deletions packages/bootstrap-vue-next/src/components/BApp/BApp.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
<template>
<ConditionalTeleport :to="teleportTo" :disabled="teleportDisabled">
<BAppOrchestrator
v-if="!noOrchestrator && (!noModals || !noToasts || !noPopovers)"
:append-toast="appendToast"
:no-popovers="noPopovers"
:no-toasts="noToasts"
:no-modals="noModals"
/>
</ConditionalTeleport>
<slot v-bind="$attrs" />
</template>

<script setup lang="ts">
import {
type ComponentInternalInstance,
computed,
inject,
provide,
readonly,
type Ref,
ref,
} from 'vue'
import {
breadcrumbGlobalIndexKey,
breadcrumbPluginKey,
defaultsKey,
globalShowHideStorageInjectionKey,
modalControllerPluginKey,
modalManagerPluginKey,
popoverPluginKey,
type RegisterShowHideFnInput,
type RegisterShowHideMapValue,
rtlPluginKey,
toastPluginKey,
} from '../../utils/keys'
import type {BvnComponentProps} from '../../types/BootstrapVueOptions'
import type {BAppProps} from '../../types/ComponentProps'
import type {
ModalOrchestratorArrayValue,
PopoverOrchestratorArrayValue,
ToastOrchestratorArrayValue,
} from '../../types/ComponentOrchestratorTypes'
import BAppOrchestrator from './BAppOrchestrator.vue'
import ConditionalTeleport from '../ConditionalTeleport.vue'
import type {BreadcrumbItemRaw} from '../../types/BreadcrumbTypes'

defineOptions({
inheritAttrs: false,
})

const props = withDefaults(defineProps<BAppProps>(), {
noModals: false,
noToasts: false,
noPopovers: false,
inherit: false,
appendToast: false,
teleportTo: undefined,
defaults: undefined,
mergeDefaults: false,
deepMerge: false,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mergeDefaults is boolean | function to where the function is the algorithm to use for merging?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describ F438 e this comment to others. Learn more.

I generally dislike props that just effect the behavior of other props. Begs the question why it's not just an object

noOrchestrator: false,
rtl: false,
})

// Inject existing defaults from parent (could be from plugins)
const injectedDefaults = inject(defaultsKey, ref({}))

function deepMerge(
target: Record<string, unknown>,
source: Record<string, unknown>
): Record<string, unknown> {
for (const key in source) {
if (
source[key] instanceof Object &&
target[key] instanceof Object &&
!Array.isArray(source[key]) &&
!Array.isArray(target[key])
) {
deepMerge(target[key] as Record<string, unknown>, source[key] as Record<string, unknown>)
} else {
target[key] = source[key]
}
}
return target
}

// Merge injected defaults with prop defaults (prop takes priority)
const mergedDefaults = computed(() => {
const merged = {...injectedDefaults.value}

if (props.defaults) {
if (props.mergeDefaults) {
if (props.deepMerge) {
deepMerge(merged, props.defaults)
} else {
Object.assign(merged, props.defaults)
}
} else {
return props.defaults
}
}

return merged
})

// Provide the merged defaults to child components
provide(defaultsKey, ref(mergedDefaults.value) as Ref<Partial<BvnComponentProps>>)

const teleportDisabled = computed(() => props.teleportTo === undefined)

const popoverController = inject(popoverPluginKey, undefined)
const toastController = inject(toastPluginKey, undefined)
const modalController = inject(modalControllerPluginKey, undefined)

if (!props.noPopovers) {
if (popoverController && props.inherit) {
provide(popoverPluginKey, popoverController)
} else {
provide(popoverPluginKey, {
popovers: ref<PopoverOrchestratorArrayValue[]>([]),
_isOrchestratorInstalled: ref(false),
})
}
}

if (!props.noToasts) {
if (toastController && props.inherit) {
toastController._isOrchestratorInstalled.value = true
provide(toastPluginKey, toastController)
} else {
provide(toastPluginKey, {
toasts: ref<ToastOrchestratorArrayValue[]>([]),
_isAppend: ref(false),
_isOrchestratorInstalled: ref(false),
})
}
}
if (!props.noModals) {
if (modalController && props.inherit) {
modalController._isOrchestratorInstalled.value = true
provide(modalControllerPluginKey, modalController)
} else {
provide(modalControllerPluginKey, {
modals: ref<ModalOrchestratorArrayValue[]>([]),
_isOrchestratorInstalled: ref(false),
})
}
}

const showHideStorage = inject(globalShowHideStorageInjectionKey, undefined)
if (!showHideStorage) {
const values: Ref<Map<string, RegisterShowHideMapValue>> = ref(new Map())

const register = ({
id,
component,
value,
toggle,
show,
hide,
registerTrigger,
unregisterTrigger,
}: RegisterShowHideFnInput) => {
values.value.set(id, {
id,
component,
value: readonly(value),
toggle,
show,
hide,
registerTrigger,
unregisterTrigger,
})
return {
unregister() {
// delete values.value[id]
values.value.delete(id)
},
updateId(newId: string, oldId: string) {
const existingValue = values.value.get(oldId)
if (existingValue) {
values.value.set(newId, {...existingValue, id: newId})
values.value.delete(oldId)
}
},
}
}
provide(globalShowHideStorageInjectionKey, {register, values})
}

const modalManager = inject(modalManagerPluginKey, undefined)
if (!modalManager) {
const stack: Ref<Map<number, ComponentInternalInstance>> = ref(new Map())

const countStack = computed(() => stack.value.size)
const valuesStack = computed(() => [...stack.value.values()])
const lastStack = computed(() => valuesStack.value[valuesStack.value.length - 1])

const pushStack = (modal: Readonly<ComponentInternalInstance>) => {
stack.value.set(modal.uid, modal)
}
const removeStack = (modal: Readonly<ComponentInternalInstance>) => {
stack.value.delete(modal.uid)
}

/**
* A collection of all registered modals
*/
const registry: Ref<Map<number, ComponentInternalInstance>> = ref(new Map())

// Utility getters not made, would not be used (count, last)

const pushRegistry = (modal: Readonly<ComponentInternalInstance>) => {
registry.value.set(modal.uid, modal)
}
const removeRegistry = (modal: Readonly<ComponentInternalInstance>) => {
registry.value.delete(modal.uid)
}

provide(modalManagerPluginKey, {
countStack,
lastStack,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
registry: readonly(registry) as Readonly<Ref<Map<number, ComponentInternalInstance>>>,
stack: valuesStack,
pushStack,
removeStack,
pushRegistry,
removeRegistry,
})
}
Comment on lines +192 to +233
Copy link
@coderabbitai coderabbitai bot Jun 18, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Modal manager reactivity is broken due to non-reactive Map

ref(new Map()) does not track internal .set/.delete mutations, so countStack, valuesStack, and lastStack will never update.

Convert to reactive(new Map()) or recreate the map on mutation:

-const stack: Ref<Map<number, ComponentInternalInstance>> = ref(new Map())
+const stack = reactive(new Map<number, ComponentInternalInstance>())

Same issue for registry.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/bootstrap-vue-next/src/components/BApp/BApp.vue between lines 185
and 226, the modal manager uses ref(new Map()) for stack and registry, but
mutations like .set and .delete on Map are not reactive with ref. To fix this,
replace ref(new Map()) with reactive(new Map()) for both stack and registry,
ensuring that changes to the Map trigger reactivity and update computed
properties like countStack, valuesStack, and lastStack correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@VividLemon is this true or not? if it's true it's also a bug in the current implementation

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


const breadcrumb = inject(breadcrumbPluginKey, undefined)
if (!breadcrumb) {
const items = ref<Record<string, BreadcrumbItemRaw[]>>({
[breadcrumbGlobalIndexKey]: [],
})
const reset = (key: string = breadcrumbGlobalIndexKey) => {
items.value[key] = []
}

provide(breadcrumbPluginKey, {items, reset})
}

const rtl = inject(rtlPluginKey, undefined)
if (!rtl) {
const rtlDefault = false
const localeDefault = undefined

< 6688 /span>
const rtlInitial =
typeof props.rtl === 'boolean' ? rtlDefault : (props.rtl?.rtlInitial ?? rtlDefault)

const localeInitial =
typeof props.rtl === 'boolean' ? localeDefault : (props.rtl?.localeInitial ?? localeDefault)

const isRtl = ref(rtlInitial)
const locale = ref(localeInitial)

provide(rtlPluginKey, {isRtl, locale})
}
</script>
Loading
Loading
0