Skip to content

fix(Popover): correct fallback offset value to 8px when no arrow#4931

Open
olros wants to merge 2 commits into
digdir:mainfrom
olros:fix/popover-no-arrow-offset
Open

fix(Popover): correct fallback offset value to 8px when no arrow#4931
olros wants to merge 2 commits into
digdir:mainfrom
olros:fix/popover-no-arrow-offset

Conversation

@olros
Copy link
Copy Markdown

@olros olros commented May 28, 2026

Summary

If the popover/tooltip/some other element depending on popover.ts is created with custom styles not using a ::before pseudo-element, the shift-param currently ends up being 0, even though the comment at the same line implies that it should default to 8px. This fixes that.

Checks

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 28, 2026

🦋 Changeset detected

Latest commit: 5874af6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@digdir/designsystemet-web Patch
@digdir/designsystemet Patch
@digdir/designsystemet-css Patch
@digdir/designsystemet-types Patch
@digdir/designsystemet-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment thread .changeset/heavy-pots-knock.md Outdated
"@digdir/designsystemet-web": patch
---

**Popover**: Correct the fallback offset floating value to 8px when no arrow is present
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This should affect all floating elements using popover.ts I think, not sure if you want the changeset to specify all of them?

@eirikbacker
Copy link
Copy Markdown
Contributor

Thanks for PR @olros 🤗 and good point! 🌟
This should probably instead calculate the value of a custom CSS property instead of always falling back to 8px, so it is possible to set this attribute to the size of the arrow. The popover.ts logic is used by both suggestions, dropdowns, tooltips, and popover components, as they all use the popover attribute. 8px arrows are not always the case, and we want all numbers to be configurable, so we can go ahead and iterate on this ☺️

Comment thread .changeset/heavy-pots-knock.md Outdated
@mimarz
Copy link
Copy Markdown
Collaborator

mimarz commented Jun 3, 2026

Thanks for PR @olros 🤗 and good point! 🌟 This should probably instead calculate the value of a custom CSS property instead of always falling back to 8px, so it is possible to set this attribute to the size of the arrow. The popover.ts logic is used by both suggestions, dropdowns, tooltips, and popover components, as they all use the popover attribute. 8px arrows are not always the case, and we want all numbers to be configurable, so we can go ahead and iterate on this ☺️

Thanks for the PR @olros!
A small change but it lays the seed for bigger improvements! Love it!

I have made a separate issue for work on the suggested improvements from @eirikbacker. We have a long list of features we are currently working on, and going into summer we are unsure when we'll get to have a look at this.

I've made an issue for it so anyone can have a go at it when there is time :)

#4940

@olros olros force-pushed the fix/popover-no-arrow-offset branch from 95dfabd to 5874af6 Compare June 3, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants