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

DataTable: Selection semantic for disabled rows #11290

Closed
pfumi opened this issue Jan 16, 2024 · 9 comments · Fixed by #11294
Closed

DataTable: Selection semantic for disabled rows #11290

pfumi opened this issue Jan 16, 2024 · 9 comments · Fixed by #11294
Assignees
Labels
🐞 defect Bug...Something isn't working
Milestone

Comments

@pfumi
Copy link
Contributor

pfumi commented Jan 16, 2024

Describe the bug

Using DataTable with a predefined selection causes multiple problems when there are items which are disabled with disabledSelection.

Issue 1

The "select all" checkbox is not correctly checked when there are checked disabled items.

For example:
1 row is checked
1 row (disabled) is checked
1 row is unchecked
1 row (disabled) is unchecked

The "select all" checkbox for this example is checked, even if there is 1 row (enabled) which is unchecked.

Issue 2

It is possible to tab on a disabled checkbox, the focus therefore is then visible.

Issue 3

If the DataTable is submitted with a disabled row which is checked, then the selection on serverside is updated and dismisses the disabled checked row.

Reproducer

Issue 1

  1. Start the reproducer primefaces-14.0.0-test-master.zip or primefaces-test-master.zip
  2. The "select all" checkbox is checked, but there is 1 row (enabled) which is unchecked

Issue 2

  1. Start the reproducer primefaces-14.0.0-test-master.zip or primefaces-test-master.zip
  2. Click on the checkbox of the first row
  3. Tab on the next row
  4. The disabled checkbox is focused

Issue 3

  1. Start the reproducer primefaces-14.0.0-test-master.zip or primefaces-test-master.zip
  2. The first row (enabled) is checked, and the second row (disabled) is checked
  3. Click on the button "Submit"
  4. The first row is still checked, but the second row is unchecked

Expected behavior

Issue 1

The "select all" checkbox should only be checked, if all enabled checkboxes are checked

Issue 2

A disabled checkbox should not be focusable

Issue 3

If there is a predefined selection with a row which is a disabled checked row, the selection of the disabled checked row should not me dismissed

PrimeFaces edition

Community, Elite

PrimeFaces version

14.0.0-SNAPSHOT, 13.0.4

Theme

No response

JSF implementation

Mojarra

JSF version

2.2

Java version

8

Browser(s)

No response

@pfumi pfumi added ‼️ needs-triage Issue needs triaging 🐞 defect Bug...Something isn't working labels Jan 16, 2024
@melloware
Copy link
Member

Just clarifying is this a new issue in 13? Did it work in previous PF versions?

@melloware melloware removed the ‼️ needs-triage Issue needs triaging label Jan 16, 2024
@pfumi
Copy link
Contributor Author

pfumi commented Jan 16, 2024

Let me check...

@pfumi
Copy link
Contributor Author

pfumi commented Jan 16, 2024

This issue #9552 changed the behavior (issue 3)
Just testet it with 12.0.0, I have no 12.0.x elite version, there is the same behavior for issue 1 and 2, but issue 3 is working.

@melloware
Copy link
Member

OK I have a fix for 1 and 2.

@melloware
Copy link
Member

melloware commented Jan 16, 2024

Your issue #3 is tricky and i think the current behavior is correct?

Because if a HTML checkbox is disabled it does not send its value in a form submit. I think the previous behavior in 12.0.0 was actually incorrect/bug and if its disabled it should not be submitted.

HTML Disabled form elements are not submitted with the form.

@pfumi
Copy link
Contributor Author

pfumi commented Jan 16, 2024

I know an HTML checkbox behaves this way, but how can I solve following use case:
I define a selection with values and certain values should not be unchecked, so I use the disabledSelection feature.
Additional to the disabled checked rows I want to select other rows. All selected rows in my opinin should then be selected, also disabled ones.
Any workaround I can use to achieve this behavior?

melloware added a commit to melloware/primefaces that referenced this issue Jan 16, 2024
melloware added a commit to melloware/primefaces that referenced this issue Jan 16, 2024
@melloware melloware added the discussion Item needs to be discussed by core devs label Jan 16, 2024
@melloware
Copy link
Member

The only way I can think of is to leave them enabled but write some JS code that sets them ui-state-disabled so they look disabled but are still the normal checkboxes and outside of disabledSelection values so they get treated like normal selections?

melloware added a commit to melloware/primefaces that referenced this issue Jan 16, 2024
@melloware melloware removed the discussion Item needs to be discussed by core devs label Jan 16, 2024
@pfumi
Copy link
Contributor Author

pfumi commented Jan 17, 2024

@melloware
I think the behaviour of issue #3 is due to implementation details of the
primefaces datatable and the HTML-standard does not justify this behaviour.

To demonstrate this let's look at two possible implementations of selections
in datatables:

  1. Using datatable-internal checkboxes (via the attribute "selection"):
    Here disabled checkboxes lose their value on form submit.
  2. Using a p:column in the datatable with explicit checkboxes:
    Here disabled checkboxes do not lose their value on form submit.

The problem in 1) is not that the checkbox is not submitted (this is HTML
standard and perfectly fine) but that it is removed from the list of selected
items (this is an implementation detail and imho even violates the HTML-standard).

@melloware
Copy link
Member

I see what you are saying but it was done in #9552 to prevent the exact behavior you are saying you need. In My opinion disabled checkboxes should not update the backend.

@tandraschko @jepsar @christophs78 @Rapster any thoughts here?

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
Development

Successfully merging a pull request may close this issue.

2 participants