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

Allow Floating Strategy to be Configurable for Popover #1504

Open
2 tasks done
sean-redteam opened this issue Oct 25, 2024 · 8 comments
Open
2 tasks done

Allow Floating Strategy to be Configurable for Popover #1504

sean-redteam opened this issue Oct 25, 2024 · 8 comments
Labels
discussion It is not a bug or an enhancement

Comments

@sean-redteam
Copy link

  • I have searched the Issues to see if this bug has already been reported
  • I have tested the latest version

Summary

Describe how it should work, and provide examples of the solution, which might include screenshots or code snippets.

Hello and thanks for providing this reference implementation for Flowbite. I have a use case for the Popover that is not currently covered but can easily be added.

I am using a Popover component inside of a parent which is overflow: auto set. This means the popover is cut off. See screen shot.

Screenshot 2024-10-25 at 11 57 19 AM

This is due to @floating-ui/react using position: absolute by default.

The good news is that useFloating has a strategy prop that can be passed to override this if necessary. In scenarios like mine, position: fixed is needed. I did some testing, and it works well. See screen shot.

Screenshot 2024-10-25 at 11 59 55 AM

The popover is still attached to the reference element and scrolls along with it.

Context

What are you trying to accomplish? How is your use case affected by not having this feature?

Popover should be updated to accept a strategy prop of type:

strategy?: 'absolute' | 'fixed';

This is passed to useFloating from the @floating-ui/react library.

Without this feature, popovers are cut off in parents that have overflow other than visible.

I'm happy to create a PR for this if it will help.

@rluders
Copy link
Collaborator

rluders commented Oct 25, 2024

@sean-redteam is it supported by standard Flowbite library?

@SutuSebastian
Copy link
Collaborator

@sean-redteam is it supported by standard Flowbite library?

Standard Flowbite library does not not use "floating-ui"

@sean-redteam
Copy link
Author

sean-redteam commented Oct 25, 2024

@rluders It could be. It's using Popper which has been migrated to Floating UI. That's what this library uses.

My assumption is the strategy wasn't implemented explicitly with position: absolute. Seems like it's just using the default strategy. It's why I didn't create this issue as a bug. This is more finding a new use case for the popover that requires setting a non-default strategy.

Without this update, it won't be possible to use popovers inside elements with overflow values other than visible. That's a bit limiting.

@rluders
Copy link
Collaborator

rluders commented Oct 25, 2024

TBH, I don't know. @sean-redteam did you tried to achieve the same result using the component-theme system? 🤔

@sean-redteam
Copy link
Author

The styles are being added directly by Floating UI. Customizing the theme doesn't have any effect:

Screenshot 2024-10-25 at 2 03 41 PM

It's possible using !fixed which sets position: fixed !important, but that seems wrong. Floating UI already exposes a prop to configure this through its API. The more elegant solution is to add support to the component.

@rluders
Copy link
Collaborator

rluders commented Oct 25, 2024

I see... it really seems to be an easy implementation. But, as @SutuSebastian said, it isn't supported by the default Flowbite library. I'm wondering if there is any supported case in the standard library where this behavior can be supported. Maybe @zoltanszogyenyi can help us here.

See, I'm not saying "NO"... I'm saying that this is something that first needs to exists at the standard Flowbite library, if this is somehow supported, there is no reason for us to implement it. However, if it isn't supported by the standard Flowbite, we first need to see this supported somehow there or at least not causing inconsistency between the libraries.

@rluders rluders added the discussion It is not a bug or an enhancement label Oct 25, 2024
@sean-redteam
Copy link
Author

sean-redteam commented Oct 25, 2024

The standard library requires Flowbite JS for the Popover to work. The JS library is using Popper (now Floating UI). Unless I missed something, I don't believe Flowbite React uses the JS library from Flowbite. It has its own implementation; therefore, it's not something that can be added to the standard library to be picked up by this library.

Also, @SutuSebastian was just saying the standard library (JS) doesn't use Floating UI, which is technically correct since it's an older version called Popper. Floating UI = Popper v3

Relevant links:
https://github.com/themesberg/flowbite/blob/main/src/components/popover/index.ts#L2
https://popper.js.org/docs/v2/

@rluders
Copy link
Collaborator

rluders commented Oct 25, 2024

OK. So, we still wait for the standard Flowbite to support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion It is not a bug or an enhancement
Projects
None yet
Development

No branches or pull requests

3 participants