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.

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

Popover without bootstrap #563

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Popover without bootstrap #563

wants to merge 8 commits into from

Conversation

s-cork
Copy link
Collaborator

@s-cork s-cork commented Oct 2, 2024

This PR is a rewrite of the popover module
There may come a day where anvil doesn't necessarily ship with bootstrap
If that day came, then a user wouldn't be able to use the popver module if they opted out of bootstrap

This PR moves to using floating ui: https://floating-ui.com/
which is probably the way to go for this type of behaviour

I've tried to make it backwards compatible,
but i suspect this rewrite might break someone's code, so it might want to be considered as a major version bump
the classnames have changed for example, so anyone overriding the styling would break
i've gone with prefix classnames like ae-popover so that we don't conflict with random css

I've tested it on the popover example app, as well as some weird edge cases we've seen in the forum before.

Note - i've deprecated the dismiss on scroll option as floating ui supports maintaining the popover position as the page is scrolled.

@s-cork s-cork force-pushed the popover-w-bootstrap branch 2 times, most recently from bd4022d to c88743e Compare October 2, 2024 06:31
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.

2 participants