Skip to content

Fix click event bubbling in Table::onRowClick()#1655

Merged
mvorisek merged 27 commits intodevelopfrom
fix/table-row-click
Sep 7, 2022
Merged

Fix click event bubbling in Table::onRowClick()#1655
mvorisek merged 27 commits intodevelopfrom
fix/table-row-click

Conversation

@ibelar
Copy link
Copy Markdown
Contributor

@ibelar ibelar commented Aug 6, 2021 •
edited by mvorisek
Loading

fix #1601

@ibelar ibelar added the RTM label Aug 6, 2021
mvorisek
mvorisek reviewed Aug 10, 2021
View reviewed changes
@mvorisek mvorisek changed the title [feature] Allow to exclude selectors when using Table::onRowClick() Allow to exclude selectors when using Table::onRowClick() Oct 3, 2021
mvorisek
mvorisek requested changes Oct 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mvorisek mvorisek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

If I understand the issue, we need to prevent event bublibling from controls in table cells. This does not solve it, at least not automatically. Then I belive we do not even need the option to pass any selector to exlude (and for rare cases, the user can do it on it's own as the method is simple).

@mvorisek mvorisek removed the RTM label Oct 10, 2021
@mvorisek mvorisek force-pushed the fix/table-row-click branch from 35f6970 to a43c500 Compare August 28, 2022 15:25
@mvorisek mvorisek force-pushed the fix/table-row-click branch 2 times, most recently from 0192317 to 057adc3 Compare August 31, 2022 21:12
@mkrecek234
Copy link
Copy Markdown
Contributor

mkrecek234 commented Sep 1, 2022 •
edited
Loading

A thought to get this finalised: Shouldn't we add a class to all links, checkboxes or other clickable items that are added to a table row, just like you did it for Checkbox::CHECKBOX_COLUMN_CSS like atk4-table-column-clickable. This class we set automatically for classes in Atk4\Ui\Table\Column like you did for Checkboxes, but also for Link. We can then define function onRowClick($action, array $excludeSelector = ['atk4-table-column-clickable]). So developers have all flexibility.

Whatever you guys consider best to get this released ;-) Please comment and I can adjust the code.

@mkrecek234
Copy link
Copy Markdown
Contributor

mkrecek234 commented Sep 2, 2022 •
edited
Loading

We should do a generalisation by a defined class, here atk4-norowclick.
The selector td:not(atk4-norowclick) does not work, it does not prevent the action be triggered as you can see in the grid.php demo.

@mkrecek234 mkrecek234 requested a review from mvorisek September 2, 2022 07:29
@mkrecek234
Copy link
Copy Markdown
Contributor

mkrecek234 commented Sep 2, 2022 •
edited
Loading

Fixed, working now as specified - please check demo and revert demo changes in grid.php before merge

@mkrecek234 mkrecek234 added the RTM label Sep 2, 2022
@mvorisek mvorisek removed the RTM label Sep 2, 2022
mvorisek
mvorisek reviewed Sep 2, 2022
View reviewed changes
mvorisek
mvorisek reviewed Sep 2, 2022
View reviewed changes
mvorisek
mvorisek reviewed Sep 2, 2022
View reviewed changes
mvorisek
mvorisek reviewed Sep 2, 2022
View reviewed changes
mvorisek
mvorisek reviewed Sep 2, 2022
View reviewed changes
@mkrecek234 mkrecek234 changed the title Allow to exclude selectors when using Table::onRowClick() Exclude checkbox, links and classes marked atk4-norowclick from Table::onRowClick() Sep 2, 2022
@mkrecek234 mkrecek234 requested a review from mvorisek September 2, 2022 11:28
@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Sep 2, 2022

@mkrecek234 PR looks very well made!

I just need to write the test.

Also, the coverage has decreased hugely. I am already on it. The problem is present since #1840. It seems there is some bug with https://github.com/sebastianbergmann/php-code-coverage itself when caching is enabled.

@mvorisek mvorisek changed the title Exclude checkbox, links and classes marked atk4-norowclick from Table::onRowClick() Fix click event bubling in Table::onRowClick() Sep 2, 2022
@mvorisek mvorisek changed the title Fix click event bubling in Table::onRowClick() Fix click event bubbling in Table::onRowClick() Sep 2, 2022
@mkrecek234 mkrecek234 mentioned this pull request Sep 5, 2022
Closed
@mvorisek mvorisek merged commit bfbf427 into develop Sep 7, 2022
@mvorisek mvorisek deleted the fix/table-row-click branch September 7, 2022 18:58
@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Sep 7, 2022

Thank you @ibelar and @mkrecek234!

@mvorisek mvorisek mentioned this pull request Sep 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkrecek234 mkrecek234 mkrecek234 left review comments

@mvorisek mvorisek Awaiting requested review from mvorisek

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Table->onRowClick() - Ignore click on checkboxes

3 participants

@ibelar @mkrecek234 @mvorisek