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

Add master checkbox to Grid #1921

Open
wants to merge 62 commits into
base: develop
Choose a base branch
from

Conversation

mhuser
Copy link
Contributor

@mhuser mhuser commented Oct 28, 2022

fix #1877

Add a master checkbox into grid header that enables to toggle selection of whole visible records.

does not account the child checkbox states
@mhuser
Copy link
Contributor Author

mhuser commented Oct 29, 2022

I have a first working master checkbox but the code is ugly and it does not account for the indeterminate state if child checkboxes are changed.
I found this fomentic example
Would it be cleaner to use a separated javascript file?

@mvorisek mvorisek changed the title add master checkbox in grid header for addSelection Add master checkbox in grid header for addSelection Oct 29, 2022
@mvorisek
Copy link
Member

needs a Behat test

master checkbox must reflect the child states - unchecked/"partially checked"/"all checked"

if Fomantic-UI/jQuery cannot handle this in a few lines, the JS should be put into js/, but until you have fully working solution, feel free to keep the code uncompiled

@mhuser
Copy link
Contributor Author

mhuser commented Oct 29, 2022

image image

Ok, it works as of now with the indeterminate state if the child checkboxes are not all in the same state.

I had some issues to have the javascript statements to appear at the proper position. I first wrote them in Grid->addSelection() and they were called kind of before the checkbox() creation for fomantic-ui.

Still I feel the manner I use the javascript is not optimal because it does not look like other atk4/ui sources.

tests-behat/grid.feature Outdated Show resolved Hide resolved
tests-behat/grid.feature Outdated Show resolved Hide resolved
@mhuser
Copy link
Contributor Author

mhuser commented Nov 5, 2022

I did not find any figures of the checkbox column inside the Column/Checkbox or the Grid documentation.

There seem to be no documentation updates to do because the core functionalities of Grid and Column/Checkbox are not affected.

@mhuser
Copy link
Contributor Author

mhuser commented Aug 10, 2023

I have now only LF line endings, but it seems that Unit / Behat (latest, Chrome Slow) keeps some CRLF in comments, which lead diff to fail 🤔

@mvorisek
Copy link
Member

I have now only LF line endings, but it seems that Unit / Behat (latest, Chrome Slow) keeps some CRLF in comments, which lead diff to fail 🤔

disable auto crlf in your git config

and js/package.json and js/package-lock.json should have no changes, delete the lock file and restore the original deps, then run npm install

@mhuser
Copy link
Contributor Author

mhuser commented Aug 10, 2023

disable auto crlf in your git config

and js/package.json and js/package-lock.json should have no changes, delete the lock file and restore the original deps, then run npm install

Thanks, I will try it !
Can you please retrigger the failing Behat (8.0, Chrome) test?
Seems it had some execution issue and not a PR-related error.

@mhuser
Copy link
Contributor Author

mhuser commented Aug 10, 2023

DOM node changes can be detected using MutationObserver, make sure you do not watch more events than needed to keep it efficient

I was not able to improve my js skills enough to address this yet. Maybe we could merge this PR as is (when the quality will be satisfactory) and open a new issue for that cases where a row is removed/hidden from DOM or the grid is dynamically scrolled?

A preliminary test on my production server already granted me positive feedback from the users that appreciate the possibility to check all the displayed rows in a single click 😄 When the array has multiple pages, only the visible page is selected.

@mhuser
Copy link
Contributor Author

mhuser commented Aug 11, 2023

In combination with the modal bulk action #1920, this will enable to mass process of selected elements of a grid conveniently.
This needs to be merged first in such a way the other PR could implement Disable/Enable of the bulk action depending on the master checkbox state.

Then Toast display should contain text "Selected: 1,2,3,4,5#"
When I click using selector "//tr//div.ui.master.checkbox"
Then I press button "Show Selection"
Then Toast display should contain text "Selected: #"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I click using selector "//tr//div.ui.master.checkbox"
Then I press button "Show Selection"
Then Toast display should contain text "Selected: #"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1920 adds support for non-UA (atk4/data User Action) bulk support

before we finish this PR, another PR should be made to add bulk support for "regular UA for multiple records" - https://github.com/atk4/data/blob/4.0.0/src/Model/UserAction.php#L32

@mvorisek mvorisek force-pushed the add-master-checkbox-to-grid--addSelection() branch from 3dfe1b5 to 1b28c35 Compare March 30, 2024 11:32
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.

Add checkbox to table header to select/unselect all
2 participants