-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Implement checkbox option for individual file in search result panel. #8260
Conversation
@RaymondLim np, checking it out now. |
@larz0 I don't see your changes. Are you committing to this branch? |
@RaymondLim it's pretty small so I'll just push to this branch, give me 10 mins. |
@RaymondLim ok done. |
Looks great! Thanks @larz0. |
@@ -2,6 +2,7 @@ | |||
<tbody> | |||
{{#searchList}} | |||
<tr class="file-section" data-file-index="{{fileIndex}}"> | |||
{{#replace}}<td class="checkbox-column"><input type="checkbox" class="check-one-file" {{#isChecked}}checked="true"{{/isChecked}} /></td>{{/replace}} | |||
<td colspan="{{#replace}}3{{/replace}}{{^replace}}2{{/replace}}"> |
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.
This is wrong now and it might be the reason of the misalignment. It should 2 in both cases.
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.
Thanks @TomMalbran. Will push a change soon.
Triage Complete |
When all items for a file are checked on and I toggle off one of them, then the checkbox for file is turned off (as I expected). When I toggle the same item back on (so all items for file are checked), I expected that the checkbox for file would be turned back on, but it wasn't. Not sure how hard that would be to implement, but I think it would be nice to have. I see similar behavior with "header" checkbox that controls all files & items. Another unexpected behavior with header checkbox: If all checkboxes are on, and I toggle off 1 item, then header checkbox is turned off. But, if all checkboxes are turned on and I toggle off a file, the header checkbox stays on. |
@redmunds I agree with you regarding to turn back on the checkbox when all items in a file are checked. You can observe the same anomaly with the checkbox in the title. I'll try to work on both. |
@RaymondLim I added another case to this comment, so please re-read. |
When all checkboxes under a file or header are checked, the checkbox is on. If any checkbox under a file or header are off, then the checkbox is off. I haven't tried it, but there is an indeterminate state that could possibly be used in the case where some checkboxes are on and some are off to give user more feedback. This is helpful when the number of items under a file or header exceeds height of panel and you can't see them all at once. |
Done with initial review. |
@redmunds I just implement the following three cases that you mentioned.
I did try to use indeterminate state, but it does not show the indeterminate state in Brackets. So it won't be in for this pr. I also try to fix the indentation issue in file rows, but can't find a way to control the table layout with anything in CSS. @larz0 Can you take a look and see you can fix the table layout issue when all file nodes are collapsed? |
@RaymondLim Adding a width to the checkbox column should fix the width issue. And since is a table , the width can even be smaller than the actual width. The same solution is used for the line numbers column to prevent the same problem. |
Thanks @TomMalbran It works. I can even remove @larz0 tweak for file rows and just use |
@redmunds Ready for re-review. |
Looks good. Merging. |
Implement checkbox option for individual file in search result panel.
@larz0 Checkboxes for files are not aligned with others, can you take a look?