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

selectionchanged event fires on mouse move and doesn't fire at all if selectionMode is set to "row" #158

Closed
domehead100 opened this issue Aug 12, 2018 · 5 comments
Assignees
Labels

Comments

@domehead100
Copy link

Expected behavior and actual behavior.

I would expect the "selectionchanged" event to only fire when the user selects rows with the mouse (by clicking or clicking and dragging a rubber band selection rectangle) or the keyboard by holding Ctrl or Shift keys and moving to additional cells. Instead, it seems to fire every time the mouse hovers over a cell. I also don't think that it fires reliably (or maybe not at all) if I click on the row header cell to select an entire row that way when not in "row" selectionMode.

When the selectionMode is set to "row", I'd expect the selectionChanged event to fire when the user clicks on a row, but instead it never fires at all in "row" selectionMode as far as I can tell.

Steps to reproduce the problem.

Add an event listener for the "selectionchanged" event and see that it triggers basically every time you move the mouse.

Set the selectionMode to "row" and see that the handler for "selectionchanged" never fires, even if you click on a row or rows.

Specifications like the version of the project, operating system, or hardware.

Latest version, Windows 10

@TonyGermaneri
Copy link
Owner

It should only fire when the selection changes, this sounds like a bug. The event should fire when rows are selected as well.

@domehead100
Copy link
Author

Well, it seems to work correctly in, for example, http://jsfiddle.net/xz9tvbr8/1/. So, maybe there's something I'm doing that's stupid. I'll try to track this down.

It does not trigger the selectionchanged event when a row is selected, however.

@TonyGermaneri
Copy link
Owner

Still a bug then, good to know it's not completely broken.

This is the universal method that everything uses for setting individual row selection selectRow.

The embarrassingly complex mouseMove function that invokes the selectRow... and it looks to me like it has been set to not fire the event for some reason that is unbeknownst to me. self.selectRow(o.rowIndex, ctrl, null, true); (the function signature is: self.selectRow = function (rowIndex, ctrl, shift, supressEvent)) ... so looks that that's the bug, just have to set that true to false and make sure it doesn't break everything.

I really can't think of a reason why it's set to true (supressEvent), nothing internally uses the event system. The only reason I have supressEvent is to make sure things like selectAll don't cause eventListener * row.length functions to fire off, which seems like undesirable behavior for the person listening to the selectionchanged event, a select all should produce 1 event. There are other places too, like holding the shift key and selecting a range or using they keyboard to select a range with shift. All of these loop through many rows running selectRow and then manually firing the selectionchanged event after the loop.

Feel free to pick up this issue. Seems very easy to fix. Just gota make sure tests still pass.

@asolas
Copy link

asolas commented Dec 5, 2018

selectionchanged not fired when "columnHeaderClickBehavior" set to "select" too

@TonyGermaneri
Copy link
Owner

Thanks for the update.

TonyGermaneri added a commit that referenced this issue Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants