Add master checkbox to Grid #1921
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import $ from 'external/jquery'; | ||
|
|
||
| export default { | ||
| /** | ||
| * Simple helper to help displaying Fomantic-UI checkbox within an atk grid. | ||
| * The master checkbox in the header of the column enable to toggle all | ||
| * content checkboxes to check or uncheck. A partially checked master checkbox | ||
| * is displayed if appopriate. | ||
| */ | ||
| masterCheckbox: function () { | ||
| $('.table .master.checkbox').checkbox({ | ||
| // check all children | ||
| onChecked: function () { | ||
| const $childCheckbox = $(this).closest('.table').find('.child.checkbox'); | ||
| $childCheckbox.checkbox('check'); | ||
| }, | ||
| // uncheck all children | ||
| onUnchecked: function () { | ||
| const $childCheckbox = $(this).closest('.table').find('.child.checkbox'); | ||
| $childCheckbox.checkbox('uncheck'); | ||
| }, | ||
| }); | ||
| }, | ||
|
|
||
| childCheckbox: function () { | ||
| $('.table .child.checkbox').checkbox({ | ||
| // Fire on load to set parent value | ||
| fireOnInit: false, | ||
|
|
||
| // Change parent state on each child checkbox change | ||
| onChange: function () { | ||
| const $listGroup = $(this).closest('.table'); | ||
| const $parentCheckbox = $listGroup.find('.master.checkbox'); | ||
| const $checkbox = $listGroup.find('.child.checkbox'); | ||
| let allChecked = true; | ||
| let allUnchecked = true; | ||
|
|
||
| // check to see if all other siblings are checked or unchecked | ||
| $checkbox.each(function () { | ||
| if ($(this).checkbox('is checked')) { | ||
| allUnchecked = false; | ||
| } else { | ||
| allChecked = false; | ||
| } | ||
| }); | ||
| // set parent checkbox state, but don't trigger its onChange callback | ||
| if (allChecked) { | ||
| $parentCheckbox.checkbox('set checked'); | ||
| } else if (allUnchecked) { | ||
| $parentCheckbox.checkbox('set unchecked'); | ||
| } else { | ||
| $parentCheckbox.checkbox('set indeterminate'); | ||
| } | ||
| }, | ||
| }); | ||
| }, | ||
| }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,20 @@ Feature: Grid | |
| Then No toast should be displayed | ||
| Then PATCH MINK the url should match "~_unit-test/grid-rowclick.php#test~" | ||
|
|
||
| Scenario: master checkbox | ||
| Given I am on "_unit-test/grid-rowclick.php" | ||
| When I press button "Show Selection" | ||
| Then Toast display should contain text "Selected: #" | ||
| When I click using selector "//tr[1]//div.ui.child.checkbox" | ||
| Then I press button "Show Selection" | ||
| Then Toast display should contain text "Selected: 1#" | ||
| When I click using selector "//tr//div.ui.master.checkbox" | ||
| Then I press button "Show Selection" | ||
| 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: #" | ||
|
|
||
|
|
||
|
There was a problem hiding this comment. #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 |
||
| Scenario: popup column header | ||
| Given I am on "collection/tablecolumnmenu.php" | ||
| Then I should not see "Name popup" | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About 1/2 of the CI Behat jobs fail with:
I did not experienced such Behat failures on other CI jobs. We run Behat testing very frequently, so it seems related with this PR.
Google Chrome Performance tools in developer tools might help you to understand the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IPP=1000 page load is slow, probably because it checks all checkboxes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the
$grid->addSelection();, setting the IPP to 1000 is much faster.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to set
fireOnInit: false. As of now, it is not possible to have checkboxes preset on init.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better.
How fortunate this behat ipp test has been added!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the init invoked something in the master checkbox which recomputed something using the whole data, ie. O(n^2) complexity. Or some slowdown due FUI JS? In general, 1k rows is no heavy workload, so it should be fast even with 10k rows :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, each checkbox controls all the other checkboxes to see if the master checkbox has to be changed, which is very inefficient. The default is unchecked for all, and cannot be changed yet so this check has no sense.