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 clickable row option for grid actions #15859

Merged
merged 6 commits into from Oct 11, 2019

Make row action code more readable, and manage confirm message on row…

… click
  • Loading branch information
jolelievre committed Oct 8, 2019
commit 8a476f263643514b3feef9c9a8bfcd830da20634
@@ -35,29 +35,54 @@ export default class LinkRowActionExtension {
* @param {Grid} grid
*/
extend(grid) {
$('tr', grid.getContainer()).each(function () {
this.initRowLinks(grid);
this.initConfirmableActions(grid);
}

/**
* Extend grid
*
* @param {Grid} grid
*/
initConfirmableActions(grid) {
grid.getContainer().on('click', '.js-link-row-action', (event) => {
const confirmMessage = $(event.currentTarget).data('confirm-message');

if (confirmMessage.length && !confirm(confirmMessage)) {
event.preventDefault();
}
});
}

/**
* Add a click event on rows that matches the first link action (if present)
*
* @param {Grid} grid
*/
initRowLinks(grid) {
$('tr', grid.getContainer()).each(function initEachRow() {
const $parentRow = $(this);

$('.js-link-row-action[data-clickable-row=1]:first', $parentRow).each(function () {
const $rowLink = $(this);
$('.js-link-row-action[data-clickable-row=1]:first', $parentRow).each(function propagateFirstLinkAction() {
const $rowAction = $(this);
const $parentCell = $rowAction.closest('td');

const $parentCell = $rowLink.closest('td');
/*
* Only search for cells with non clickable contents to avoid conflicts with
* previous cell behaviour (action, toggle, ...)
*/
const clickableCells = $('td.data-type, td.identifier-type, td.badge-type', $parentRow)
.not($parentCell)
;

clickableCells.addClass('cursor-pointer').click(() => {
This conversation was marked as resolved by PierreRambaud

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Oct 8, 2019

Contributor

What if you click on a checkbox, or a link in the cell?

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 8, 2019

Author Contributor

Which is why I only apply the click on some of the cells: td.data-type, td.identifier-type, td.badge-type This way toggle cells, checkboxes or actions still work correctly

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Oct 8, 2019

Contributor

Can you add a little comment to say that?
And are you sure with theses type you're not able to add link? :trollface:

Other solution could be to get the clicked element and be sure it's a td or a text.

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 8, 2019

Author Contributor

Yes link are added using a LinkColumn to the definition which includes a link-type cell
So if you choose not to use the correct type, and override an existing template to include a link where there shouldn't be one... then you probably deserve it!!

I tried to check the event at first but the behaviour was "random" sometimes there's a div, sometimes it's the td which trigger the event, when it's a button you have other use cases Anyway it resulted in overly complex checks that still would allow false positive whereas this method is simpler to understand and (I think) guaranties the expected behaviour (we can always add additional types if need be)

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 8, 2019

Author Contributor

Comment added

document.location = $rowLink.attr('href');
const confirmMessage = $rowAction.data('confirm-message');

if (!confirmMessage.length || confirm(confirmMessage)) {
document.location = $rowAction.attr('href');
}
});
});
});
This conversation was marked as resolved by sarjon

This comment has been minimized.

Copy link
@sarjon

sarjon Oct 8, 2019

Contributor

It would be nice to wrap it into more meaningful function.

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 8, 2019

Author Contributor

Yep, done


grid.getContainer().on('click', '.js-link-row-action', (event) => {
const confirmMessage = $(event.currentTarget).data('confirm-message');

if (confirmMessage.length && !confirm(confirmMessage)) {
event.preventDefault();
}
});
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.