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 panel with input button ( and Bootstrap ) #850

Open
MrAolen opened this issue Jan 23, 2024 · 3 comments
Open

Popover panel with input button ( and Bootstrap ) #850

MrAolen opened this issue Jan 23, 2024 · 3 comments

Comments

@MrAolen
Copy link

MrAolen commented Jan 23, 2024

Hello,

I have the following issue but I don't know if it's a bug from fritz2 or a bad development.

Description :

I'd like to create a dropdown with a list so that I can select from a list of items. I had the restriction of using Boostrap for the CSS part. But I've run into compatibility problems.

In Boostrap we can use Floating Labels to make our forms a bit more dynamic.

In theory, I wanted to turn my popOverButton into an input text, which would display the drop-down list when clicked. So far so good, but when I dismiss I get a focus reset, which puts the mouse cursor back in the input.

Do you have any idea where this might be coming from?

example

To Reproduce :

I created a repository with some code to have an example.

  1. Run the app ( ./gradlew jsRun )
  2. Open the app
  3. Click on input
  4. Click outside of the input

Desktop :

  • OS: iOS 14.2.1
  • Browser Chrome
  • Version 120.0.6099.234

Thanks in advance for your help.

Best regards

@Lysander
Copy link
Collaborator

I think this is due to the focustrap-Call in the render-Code of PopOver:

    fun render() {
        attr("id", componentId)
        trapFocusWhenever(opened) // this call relies on the default params, see below
    }

Whereas the function definition is as follows:

fun Tag<HTMLElement>.trapFocusWhenever(
    condition: Flow<Boolean>,
    restoreFocus: Boolean = true, // This will put the focus back!
    setInitialFocus: InitialFocus = TryToSet
) {

I would suggest you "clone" the PopOver-component into your project (copy and paste the original code and modify the name -> MyPopOver or alike) and try to change the call with better suited values.

If that works, we could discuss, whether and how we open up the PopOver-API to let the user tune the default behaviour there.

I have the idea, that you want to build some "filtering select-menu" or "ComboBox"?
There is an open issue #674 in order to provide this btw. It might be better suited to have some dedicated headless component to adress such components. You are welcome to help us with that :-)

A small extra comment to your code:

        popOver {
            div("form-floating") {
                popOverButton("form-control border border-2", tag = RenderContext::input) {
                    id("searchFruitInput") // this is not so good! 

Pass the id into the main component factory popOver! All the brick-factories below chose their Ids based upon the parents Id. In order to set the correct label.for value, just save the popOverButton into some var to grab it from there or reconstruct it by the main componentId, that is in scope of the main factory - the latter might be more "brittle".

You may have a look at the headless components implementations. Iirc we use the var-pattern there quite a lot for handling relations (for labels and some ARIA-attributes too).

@MrAolen
Copy link
Author

MrAolen commented Feb 14, 2024

Hello,

Thank you for your information and sorry for the late reply.

By following your instructions I was able to correct the focus problem using the following code:

fun RenderContext.popOver(
    restoreFocus: Boolean = true,
    classes: String? = null,
    id: String? = null,
    scope: (ScopeContext.() -> Unit) = {},
    initialize: PopOver<HTMLDivElement>.() -> Unit
): Tag<HTMLDivElement> {
    addComponentStructureInfo("popOver", this@popOver.scope, this)
    return RenderContext::div.invoke(this, classes(classes, PopUpPanel.POPUP_RELATIVE), id, scope) {
        PopOver(this, id).run {
            initialize(this)
            attr("id", componentId)
            trapFocusWhenever(opened, restoreFocus = restoreFocus)
        }
    }
}

I have also started to try to create a specific component for this part. It's starting to work but I'm still waiting to discuss about it with my colleagues.

What's best for the next step : to create a PR and discuss about it or to discuss it beforehand and do an initial review before the PR?

@Lysander
Copy link
Collaborator

I have also started to try to create a specific component for this part. It's starting to work but I'm still waiting to discuss about it with my colleagues.

Nice!

What's best for the next step : to create a PR and discuss about it or to discuss it beforehand and do an initial review before the PR?

I would suggest to first think a bit more about the topic. As your use case is some rather complex, dedicated Combobox, I would deny the need to change the PopOver component. I would suggest to drop the PopOver inside your approach and to set up directly upon the PopUpPanel-class, as other Popups like the PopOver-component do. You can bypass such intentional limitations then.

If it is useful to provide such a parameter is imho a separate discussion and would be a separate PR imho. We need defintely to find use cases for PopOver-applications, where this behaviour is needed. Unless we find some reasonable use case, I would prefer to keep the API as simple as possible.

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

No branches or pull requests

2 participants