Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(BPopover and Btooltip): Fixes bootstrap-vue-next#1232 - do not create a new app fro each tooltip or popover #1837

Merged
merged 3 commits into from
Apr 4, 2024
Merged

fix(BPopover and Btooltip): Fixes bootstrap-vue-next#1232 - do not create a new app fro each tooltip or popover #1837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b36edcc
fix(BPopover and Btooltip): Fixes bootstrap-vue-next#1232 - do not cr…
james-muriithi Apr 3, 2024
78addbf
cleanup
xvaara Apr 4, 2024
f8a48b7
cleanup
xvaara Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 5 additions & 17 deletions packages/bootstrap-vue-next/src/directives/BPopover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {type Directive, ref} from 'vue'
import {type Directive} from 'vue'
import {
bind,
type ElementWithPopper,
Expand All @@ -16,12 +16,10 @@ export default {
const text = resolveContent(binding.value, el)

if (!text.content && !text.title) return

el.$__state = ref({
bind(el, binding, {
...resolveDirectiveProps(binding, el),
...text,
})
bind(el, binding)
},
updated(el, binding) {
const isActive = resolveActiveStatus(binding.value)
Expand All @@ -31,21 +29,11 @@ export default {

if (!text.content && !text.title) return

if (!el.$__state) {
// Same binding as above
// This happens when mounting occurs, but binding does not happen ie (if (!text.content && !text.title) return)
// So mounting occurs without a title or content set
el.$__state = ref({
...resolveDirectiveProps(binding, el),
...text,
})
bind(el, binding)
return
}
el.$__state.value = {
unbind(el)
bind(el, binding, {
...resolveDirectiveProps(binding, el),
...text,
}
})
},
beforeUnmount(el) {
unbind(el)
Expand Down
25 changes: 6 additions & 19 deletions packages/bootstrap-vue-next/src/directives/BTooltip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {type Directive, ref} from 'vue'
import {type Directive} from 'vue'
import {
bind,
type ElementWithPopper,
Expand All @@ -16,14 +16,13 @@
const text = resolveContent(binding.value, el)

if (!text.content && !text.title) return

el.$__state = ref({

Check warning on line 19 in packages/bootstrap-vue-next/src/directives/BTooltip.ts

View workflow job for this annotation

GitHub Actions / test-lint 

Delete `····`
bind(el, binding, {
noninteractive: true,
...resolveDirectiveProps(binding, el),
title: text.title ?? text.content ?? '',
tooltip: isActive,
})
bind(el, binding)
},
updated(el, binding) {
const isActive = resolveActiveStatus(binding.value)
Expand All @@ -32,26 +31,14 @@
const text = resolveContent(binding.value, el)

if (!text.content && !text.title) return
unbind(el)

if (!el.$__state) {
// Same binding as above
// This happens when mounting occurs, but binding does not happen ie (if (!text.content && !text.title) return)
// So mounting occurs without a title or content set
el.$__state = ref({
noninteractive: true,
...resolveDirectiveProps(binding, el),
title: text.title ?? text.content ?? '',
tooltip: isActive,
})
bind(el, binding)
return
}
el.$__state.value = {
bind(el, binding, {
noninteractive: true,
...resolveDirectiveProps(binding, el),
title: text.title ?? text.content ?? '',
tooltip: isActive,
}
})
},
beforeUnmount(el) {
unbind(el)
Expand Down
14 changes: 5 additions & 9 deletions packages/bootstrap-vue-next/src/utils/floatingUi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type {Placement} from '@floating-ui/vue'
export {autoUpdate} from '@floating-ui/vue'

import {type App, createApp, type DirectiveBinding, h, type Ref} from 'vue'
import {type DirectiveBinding, h, render} from 'vue'
import {DefaultAllowlist, sanitizeHtml} from './sanitizer'
import BPopover from '../components/BPopover.vue'
import type {BPopoverProps} from '../types'

// TODO this function doesn't currently resolve with RTL in mind. Once Bootstrap finalizes their RTL, we should make this change here
/**
Expand Down Expand Up @@ -106,26 +107,21 @@
})

export interface ElementWithPopper extends HTMLElement {
$__state?: Ref<{title: string; target: HTMLElement}>
$__app?: App
$__element?: HTMLElement
}

export const bind = (el: ElementWithPopper, binding: Readonly<DirectiveBinding>) => {
export const bind = (el: ElementWithPopper, binding: Readonly<DirectiveBinding>, props: BPopoverProps) => {

Check warning on line 113 in packages/bootstrap-vue-next/src/utils/floatingUi.ts

View workflow job for this annotation

GitHub Actions / test-lint 

Replace `el:·ElementWithPopper,·binding:·Readonly<DirectiveBinding>,·props:·BPopoverProps` with `⏎··el:·ElementWithPopper,⏎··binding:·Readonly<DirectiveBinding>,⏎··props:·BPopoverProps⏎`
const div = document.createElement('span')
if (binding.modifiers.body) document.body.appendChild(div)
else if (binding.modifiers.child) el.appendChild(div)
else el.parentNode?.insertBefore(div, el.nextSibling)
el.$__app = createApp({render: () => h(BPopover, {...el.$__state?.value})})
el.$__app.mount(div)
render(h(BPopover, props), div)
el.$__element = div
}

export const unbind = (el: ElementWithPopper) => {
const div = el.$__element
el.$__app?.unmount()
delete el.$__app
delete el.$__state
if (div) render(null, div)
setTimeout(() => {
div?.remove()
}, 0)
Expand Down