zotero / zotero-connectors

Chrome, Firefox, Edge, and Safari extensions for Zotero
https://www.zotero.org/download/connectors
Other
533 stars 124 forks source link

added collections search to save popup #462

Closed abaevbog closed 8 months ago

abaevbog commented 9 months ago

Replicated parts of the collection filtering logic from collectionTree. Added a filter field above the collection rows. On input, the rows are re-rendered to only include rows matching the filter (and their parents).

Fixes: #458

adomasven commented 9 months ago

Does not expand matching subcollections. Otherwise looks good.

abaevbog commented 9 months ago

Gotcha - I just added a tweak to expand all collections during filtering and if the filter is cleared, this will restore the expanded status of rows to what it was before filtering started.

The collection search in collectionTree does a few additional things once the filter is cleared: restore the initial scroll position if the selected row did not change or scroll a newly selected row into the middle. I didn't add it here because I assumed it's not that important (since once the collection is found, the popup is just closed) but correct me if I'm wrong and we want it.

dstillman commented 9 months ago

Nice!

  1. There should be an X button to clear the search when there's input.
  2. Pressing Esc in the filter box should clear it and keep focus in the box without closing the popup. I guess if you press Esc with no input, the popup should close?
  3. If you select a collection, filter for something that doesn't match, and then press Tab, it should select the first matching row (but not its parent library if the library itself doesn't match), like we do in the client. Right now the selection stays on the hidden collection. We shouldn't change the selection until you Tab into the tree, though.
abaevbog commented 8 months ago

There should be an X button to clear the search when there's input.

All the search fields from the desktop app use search-textbox which provides that X icon. I tried search-textbox but it must not be available here, so I just added an svg and placed the button in the end of the input. I think it's looking OK!

Pressing Esc in the filter box should clear it and keep focus in the box without closing the popup.

Makes sense! Escape from non-empty input clears the filter, otherwise it's just an ordinary Escape keypress that closes the popup.

If you select a collection, filter for something that doesn't match, and then press Tab, it should select the first matching row

That is done as well. Something to note: visually, there's no way to tell if a row is passing the filter of if it's a parent of a passing child. I don't think we have an equivalent of a context row here, do we?

Also, I added a tweak to expand the rows of a selected collection on tab from the collection filter or after the filter is cleared. My idea was to make sure that the selected row is always visible. It's in a separate last commit, in case if it's not necessary.

adomasven commented 8 months ago

All the search fields from the desktop app use search-textbox which provides that X icon. I tried search-textbox but it must not be available here, so I just added an svg and placed the button in the end of the input. I think it's looking OK!

Not worth bothering with svgs here, especially as separate files. Either embed it, or use something like UTF .

That is done as well. Something to note: visually, there's no way to tell if a row is passing the filter of if it's a parent of a passing child. I don't think we have an equivalent of a context row here, do we?

We don't, but it's a good idea to add it here.

Also, I added a tweak to expand the rows of a selected collection on tab from the collection filter or after the filter is cleared. My idea was to make sure that the selected row is always visible. It's in a separate last commit, in case if it's not necessary.

Not sure what this is doing? The dialog already expands collections up until the selected subcollection.

abaevbog commented 8 months ago

Not worth bothering with svgs here, especially as separate files.

Got it - i just changed it to use a utf character instead.

We don't, but it's a good idea to add it here.

Awesome, I will look into how this could be done!

Not sure what this is doing? The dialog already expands collections up until the selected subcollection.

I didn't realize there was already a piece of code doing exactly that in changeHeadline - I just moved it into a separate function expandToTarget so that we can call it after the filter is cleared. After the filter is cleared, the original collapsed status of rows before filtering is restored, and this is just an extra step to make sure that the selected row is visible if its parent was collapsed.

abaevbog commented 8 months ago

So I added a mini-context row logic to the ProgressWindow similar to itemTree. It just sets an opacity to 0.6 and the tree ends up looking like this

Screenshot 2024-03-06 at 2 25 30 PM
adomasven commented 8 months ago

The X doesn't quite look centered. Otherwise we can probably merge this, unless @dstillman has more thoughts.

dstillman commented 8 months ago

Merged in 22e1a6427e8028720fe2b6d269893d3c689fbc3a and f5c09dd266ef0f95143fb9ad768761d3b51d09e2