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

fix(ktooltip): suppress vue attr passthrough warning #2178

Merged
merged 1 commit into from
May 9, 2024
Merged

fix(ktooltip): suppress vue attr passthrough warning #2178

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 0 deletions src/components/KTooltip/KTooltip.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ import type { PopPlacements } from '@/types'
import { PopPlacementsArray } from '@/types'
import { v4 as uuidv4 } from 'uuid'
defineOptions({
inheritAttrs: false,
Copy link
Member

Choose a reason for hiding this comment

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

Would an equivalent change be to just remove the v-bind=“$attrs” on the root element since they are essentially bound by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, just removing the v-bind=“$attrs” still triggers the warning because the <slot /> is still wrapped by a <template />

image

Although we can also change the

  <template v-else>
    <slot />
  </template>

to

  <div v-else>
    <slot />
  </div>

to suppress the warning but it will introduce an extra wrapper to the slot, and the $attrs will bind to the div wrapper even for the "no-text-to-show" scenario in the tooltip

Personally, I want to avoid this behavior. I'd prefer to treat the <KToolip /> as a conditional wrapper, when the text is provided, it wraps the children and renders a popover when hovered. But if there's no text to show, it should render the children directly without any wrap. And this is the current behavior.

Please let me know if you think the "always wrap" behavior is better, cc @Leopoldthecoder since he introduced the conditional render logic here

Copy link
Member

Choose a reason for hiding this comment

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

I can provide some context: this fix is mainly about getting rid of the console warning. KCopy binds class and data-testid to KTooltip, which confuses Vue because KTooltip's root node is conditionally rendered.

})
const props = defineProps({
/**
* Text to show in tooltip
Expand Down
Loading