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

Select only visible / filtered results #13517

Closed
YoussefE123 opened this Issue Jul 24, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@YoussefE123

YoussefE123 commented Jul 24, 2017

Steps to reproduce

  1. Go to the "Browse" tab of any table
  2. Type some text in the "Filter rows" text box
  3. Click the "Check all" link at the bottom

Expected behaviour

After results are filtered, only visible / filtered results should be selected, which affects any operation from the "With selected" links, like "Edit" and "Delete" (it would be nice to have these links appear at the top as well).

Actual behaviour

All results get selected instead, even those that are hidden !
The following code seems to fix the issue : $('input[name^=rows_to_delete]:visible').prop("checked",true)

Server configuration

Operating system: Windows 7 x64

Web server: Apache 2.4.26

Database: MariaDB version 10.2.6

PHP version: 7.1.7

phpMyAdmin version: 4.7.2

@nijel nijel added the enhancement label Aug 28, 2017

@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Aug 28, 2017

Member

Indeed current situation is not intuitive, but when touching this, the behavior should be probably consistently applied in all cases - the very same applies to database structure and tables filtering.

Also it should probably behave consistently in other direction as well: Once you select all and apply filter, only filtered rows should be probably affected.

Member

nijel commented Aug 28, 2017

Indeed current situation is not intuitive, but when touching this, the behavior should be probably consistently applied in all cases - the very same applies to database structure and tables filtering.

Also it should probably behave consistently in other direction as well: Once you select all and apply filter, only filtered rows should be probably affected.

@YoussefE123

This comment has been minimized.

Show comment
Hide comment
@YoussefE123

YoussefE123 Aug 28, 2017

Hi,

I'm primarily using the "Delete" link, after filtering the results, so yes, you are right about the other cases and the "other direction" as well, but to make it simple, this only affects how JavaScript selects the checkboxes.

In fact, it has nothing to do with the logic behind actions like updating or deleting rows, as those are handled separately after the action is requested (based on the selected checkboxes).
I think the only needed change is when you click "Check All" or when the text filter changes (for the other direction), which both can be fixed in JavaScript alone like the code above (no need to touch php files, or any other database related files).

the other direction is a bit tricky, because it depends on whether the "Check All" is active or not :
1- active : apply default behavior (select only visible rows)
2- inactive : I guess de-selecting all checkboxes is enough whenever the text changes, and the user can "Check All" again if needed.

YoussefE123 commented Aug 28, 2017

Hi,

I'm primarily using the "Delete" link, after filtering the results, so yes, you are right about the other cases and the "other direction" as well, but to make it simple, this only affects how JavaScript selects the checkboxes.

In fact, it has nothing to do with the logic behind actions like updating or deleting rows, as those are handled separately after the action is requested (based on the selected checkboxes).
I think the only needed change is when you click "Check All" or when the text filter changes (for the other direction), which both can be fixed in JavaScript alone like the code above (no need to touch php files, or any other database related files).

the other direction is a bit tricky, because it depends on whether the "Check All" is active or not :
1- active : apply default behavior (select only visible rows)
2- inactive : I guess de-selecting all checkboxes is enough whenever the text changes, and the user can "Check All" again if needed.

@spyrule

This comment has been minimized.

Show comment
Hide comment
@spyrule

spyrule Aug 28, 2017

This BUG actually also affects the database table view as well. If you filter by table view, and use "check all", it selects the hidden tables as well. If you select drop, it will now delete all of the filtered AND unfiltered tables as well (found this bug by accident... wiped out 30+ tables instead of the 15 I though I had filtered/selected).

It should be noted that if you manually check each item in the view, it will only affect those items actually marked (so manually selecting the entire visible list is not affected by the same logic).

spyrule commented Aug 28, 2017

This BUG actually also affects the database table view as well. If you filter by table view, and use "check all", it selects the hidden tables as well. If you select drop, it will now delete all of the filtered AND unfiltered tables as well (found this bug by accident... wiped out 30+ tables instead of the 15 I though I had filtered/selected).

It should be noted that if you manually check each item in the view, it will only affect those items actually marked (so manually selecting the entire visible list is not affected by the same logic).

@drtothferenc19

This comment has been minimized.

Show comment
Hide comment
@drtothferenc19

drtothferenc19 Sep 25, 2017

I filtered my tables and selected the 3 results (old backup tables), clicked on delete and accidentally deleted all of my tables so it could be misleading.

drtothferenc19 commented Sep 25, 2017

I filtered my tables and selected the 3 results (old backup tables), clicked on delete and accidentally deleted all of my tables so it could be misleading.

@YoussefE123

This comment has been minimized.

Show comment
Hide comment
@YoussefE123

YoussefE123 Sep 25, 2017

Hi all,
maybe because I'm very prudent when it comes to deleting tables and databases, that these incidents never happened to me, but I do agree that this is indeed misleading.
In fact, I was deleting just a few filtered records, but in the confirmation screen, I saw a lot of DELETE commands than expected, and that's how I found out about this issue !

YoussefE123 commented Sep 25, 2017

Hi all,
maybe because I'm very prudent when it comes to deleting tables and databases, that these incidents never happened to me, but I do agree that this is indeed misleading.
In fact, I was deleting just a few filtered records, but in the confirmation screen, I saw a lot of DELETE commands than expected, and that's how I found out about this issue !

@spyrule

This comment has been minimized.

Show comment
Hide comment
@spyrule

spyrule Sep 25, 2017

I'm sorry, the fact that people can accidentally delete whole tables, should class this as a bug. Its not a code bug, but an interface design bug. Either clarify the wording, or fix the functionality to only delete the actually selected (as on screen selected) tables.

spyrule commented Sep 25, 2017

I'm sorry, the fact that people can accidentally delete whole tables, should class this as a bug. Its not a code bug, but an interface design bug. Either clarify the wording, or fix the functionality to only delete the actually selected (as on screen selected) tables.

@ankitjain28may

This comment has been minimized.

Show comment
Hide comment
@ankitjain28may

ankitjain28may Sep 29, 2017

Contributor

@nijel I would like to work on this issue, can you please assign this to me ?

Contributor

ankitjain28may commented Sep 29, 2017

@nijel I would like to work on this issue, can you please assign this to me ?

@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Oct 3, 2017

Member

Feel free to work on this. Indicating in the issue that you're working on it is usually enough.

Member

nijel commented Oct 3, 2017

Feel free to work on this. Indicating in the issue that you're working on it is usually enough.

@nijel nijel self-assigned this Oct 30, 2017

@nijel nijel added this to the 4.7.6 milestone Oct 30, 2017

@nijel nijel closed this in 7a17da0 Oct 30, 2017

nijel added a commit that referenced this issue Oct 30, 2017

Share code for row filtering
Make it generic so that it works in most cases and fixes does not have
to be implemented multiple times.

Issue #13517

Signed-off-by: Michal Čihař <michal@cihar.com>

ibennetch added a commit that referenced this issue Dec 1, 2017

Fixed check all interaction with filtering
- check all select only filtered rows
- the hidden rows are automatically unselected
- the check all state is updated according to filtering

Fixes #13517

Signed-off-by: Michal Čihař <michal@cihar.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment