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

SelectCheckboxMenu: emptyLabel not considered as overlay-target and opens behind dialog #9988

Closed
arnis87 opened this issue Apr 4, 2023 · 6 comments · Fixed by #9996 or #10036
Closed
Assignees
Labels
🐞 defect Bug...Something isn't working
Milestone

Comments

@arnis87
Copy link

arnis87 commented Apr 4, 2023

Describe the bug

Dialog bindEvents checks for whether mousedown event target was primefaces-overlay-target and if not moves to top. This causes issues with clicks on selectCheckboxMenu emptyLabel which opens the menu/dropdown but dialog moves on top of it. When clicking on dropdown arrow or actual data label it works fine.

SelectCheckboxMenu emptyLabel fails this check on line 379:

//Move dialog to top if target is not a trigger for a PrimeFaces overlay
this.jq.on("mousedown", function(e) {
if(!$(e.target).data('primefaces-overlay-target')) {
$this.moveToTop();
}
});

Reproducer

Place selectCheckboxMenu inside popup and click on emptyLabel instead of dropdown arrow.
Menu opens but container dialog moves on top right after

Expected behavior

Menu should open on top of the dialog

PrimeFaces edition

Elite

PrimeFaces version

12.0.3

Theme

No response

JSF implementation

Mojarra

JSF version

2.3

Java version

8

Browser(s)

No response

@arnis87 arnis87 added ‼️ needs-triage Issue needs triaging 🐞 defect Bug...Something isn't working labels Apr 4, 2023
@melloware
Copy link
Member

Please provide an executable example using the PrimeFaces Test project. It is the only way developers can debug your problem to help.

@melloware melloware added Status: Needs Reproducer Needs a reproducer showing the issue and removed ‼️ needs-triage Issue needs triaging labels Apr 4, 2023
@arnis87
Copy link
Author

arnis87 commented Apr 5, 2023

I will give it a go when I have more time.

For now I have debugged it myself and can see that the issue is caused by the emptyLabel being rendered without every getting the data('primefaces-overlay-target') attached to it. During init it will renderLabel after the data is attached to triggers and its children.
data('primefaces-overlay-target') is attached here:

this.triggers.data('primefaces-overlay-target', true).find('*').data('primefaces-overlay-target', true);

renderLabel() call happens few lines down.


and the emptyLabel gets constructed inside via regular html injection and no data attached
this.multiItemContainer.empty().append('<li class="ui-selectcheckboxmenu-emptylabel">' + (this.multiItemContainer.data('label') || '&nbsp;') + '</li>');

It is also issue when the emptyLabel has default value.

I have implemented a workaround/monkey patch which is to extend PrimeFaces.widget.SelectCheckboxMenu and its renderLabel method:

renderLabel: function () {
     this._super();
     this.triggers.data('primefaces-overlay-target', true).find('*').data('primefaces-overlay-target', true);
}

@melloware
Copy link
Member

Thanks for the debug. Let me submit a PR.

@melloware melloware removed the Status: Needs Reproducer Needs a reproducer showing the issue label Apr 5, 2023
@melloware melloware added this to the 13.0.0 milestone Apr 5, 2023
@melloware
Copy link
Member

Please review my PR!

@iki-hacon
Copy link

iki-hacon commented Apr 18, 2023

@melloware This bug also happens, if I klick on a multiple token label.
To solve this, the following monkey patch helped:

PrimeFaces.widget.SelectCheckboxMenu = PrimeFaces.widget.SelectCheckboxMenu.extend({

    renderLabel: function () {
        this._super();
        this.registerTrigger();
    },
    updateLabel: function () {
        this._super();
        this.registerTrigger();
    },
    /**
     * Mark trigger and descandants of trigger as a trigger for a primefaces overlay.
     * @private
     */
    registerTrigger: function() {
        if (!this.disabled) {
            this.triggers.data('primefaces-overlay-target', true).find('*').data('primefaces-overlay-target', true);
        }
    }

});

You might need to extend the fix to updateLabel also (or the rendering of the multiple tokens)

@melloware
Copy link
Member

Just submitted fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 defect Bug...Something isn't working
Projects
None yet
3 participants