-
Notifications
You must be signed in to change notification settings - Fork 106
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
base: develop
Are you sure you want to change the base?
Changes from all commits
4794b95
df9f80c
cebaf59
472db65
2542bfc
7fd9d4b
3574c18
f72140d
ce62a0a
5ed0e7a
6a837c6
8ead2ab
761c459
eb5b5ee
86be84d
5900356
52ad16e
ed8c541
e942c77
e4fa197
1b18382
31a0f25
404861f
a1a1a7a
80ac317
df582bd
337b5e9
f413c40
58bfe12
98533cf
ad89648
5cc92ab
6674fc5
9bc4968
3d6dc3f
2b3c2ca
9e018e1
3fdd93a
5a18354
7118430
024b5f9
245d5dc
da216b7
676f65d
082ab90
8fb3949
159cf9b
7166991
9536060
2e244bb
4eb182b
fe7d4ad
390cc22
8c4cc48
d85362a
a00ecac
2608a62
9469d03
ea43c69
fec1793
1b28c35
ad4909d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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'); | ||
} | ||
}, | ||
}); | ||
}, | ||
}; |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,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 selected" | ||
Then Toast display should contain text "Selected: #" | ||
When I click using selector "//tr[1]//div.ui.child.checkbox" | ||
Then I press button "Show selected" | ||
Then Toast display should contain text "Selected: 1#" | ||
When I click using selector "//tr//div.ui.master.checkbox" | ||
Then I press button "Show selected" | ||
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 selected" | ||
Then Toast display should contain text "Selected: #" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO #1920 (comment) |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
Scenario: popup column header | ||
Given I am on "collection/tablecolumnmenu.php" | ||
Then I should not see "Name popup" | ||
|
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.