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

New UI to exclude tags, chose boolean operator and limit selection to data types #9

Merged
merged 26 commits into from
Apr 25, 2024

Conversation

Tom-TBT
Copy link
Member

@Tom-TBT Tom-TBT commented Mar 19, 2024

This PR includes multiple features in parallel with code refactoring:

  • Added a choice to switch between AND & OR when combining tags.
  • Added a field (mirroring tag selection) to objects with the given tags
  • Added for each object type a tick button to include/exclude them from the result list
  • Preview checkbox to prevent sending intermediate queries to server
  • Filtering on selected user (or all members to see all)

@will-moore
Copy link
Contributor

This branch is now being deployed on merge-ci.
Functional testing looks good:

Screenshot 2024-03-22 at 14 32 00

@Tom-TBT
Copy link
Member Author

Tom-TBT commented Apr 18, 2024

Hey @will-moore
I'm done with the changes for that PR. The last two commits with changes to the HTML and tooltips should be visible tomorrow on your test instance. But all other functionalities are there already.

Since the last time you checked, I have:

  • Filtering of tags according to the selected user
  • Tagsets added to the name

If you have comments/feedback/requests, I would appreciate your input.
Otherwise, I will go forward and merge it to publish it soon. I'll be here tomorrow to discuss it.

One specific question I have is on how I implemented tag filtering for the user:
https://github.com/German-BioImaging/omero-tagsearch/pull/9/files#diff-66c29fc6bc7e94fb56ef12fa6ce7cd9de87713ee3e1f80c9aab685d81ce7c2d8R205-R221

I get the user_id from the previous work of Douglas. (I haven't yet looked at the logic of that function. It's on my todo list but I'll leave it for another PR).
Is it the "proper" way to filter tags with user_id in HQL, or is there a way to do that with service_ops or params of the query_service?

@will-moore
Copy link
Contributor

@Tom-TBT you can do

params.theFilter.ownerId = rlong(eid)

but I'm not sure if that would have quite the same behaviour as

hql += f" AND ann.details.owner.id = {user_id}"

since that explicitly filters by ann owner, whereas params may apply the ownership filter to the link, since you're doing FROM %sAnnotationLink link, so you'd have to test to check.

@pwalczysko
Copy link

All the newly added tooltips seem to have empty lines between sentences (see screenshot). Is that intentional ?
It looks a bit redundant - why the empty line ?

Screenshot 2024-04-19 at 13 45 09

@Tom-TBT
Copy link
Member Author

Tom-TBT commented Apr 19, 2024

Empty line is intentional. I'm shared between with or without, so if you find it disturbing I'll remove them. Thanks

@pwalczysko
Copy link

Empty line is intentional. I'm shared between with or without, so if you find it disturbing I'll remove them. Thanks

Understood.

Also, I like the new tooltips in comparison with the old ones - definitely an improvement, thank you.

@Tom-TBT Tom-TBT self-assigned this Apr 19, 2024
@will-moore
Copy link
Contributor

Played a bit with the filtering by Owner...
If I understand correctly, before this PR there was no filtering by owner (always operates like "All Members" does now)?

With this change there's nothing to indicate that this filtering by Tag owner has been introduced. It is consistent with other parts of webclient, but it might still catch users out.

E.g. If I'm browsing another user's data, I add a Tag that belongs to me, then I try tagsearch but I don't see the Tag I just added. This is consistent with me also not seeing my Tag if I look under the Tags tab, but in that case it's is a bit less surprising since the Tags are listed under "User-X" in the Tag tree.
Also when I browse to that user's Dataset, I can filter by my Tag (above the thumbnails panel).

@Tom-TBT
Copy link
Member Author

Tom-TBT commented Apr 19, 2024

Thanks a lot for the feedback

If I understand correctly, before this PR there was no filtering by owner (always operates like "All Members" does now)?

Yes, exectly.

With this change there's nothing to indicate that this filtering by Tag owner has been introduced.

The tooltip now has something saying that filtering happens. What do you think about the phrasing:
"Only the tags of the selected group & user are listed".
I indicated "group & user" to hint the group dropdown menu. I thought just saying user may not be enough.
"All members" doesn't appear in private groups, so if the user can find the menu, I think the "All member" option should be discoverable.
If the list of tag is empty, the tooltip seems like the obvious option to understand why it's empty.
With list of tags not matching the user's tags, that should hint to the usual OMERO group&user filtering.
And last argument, the UI has changed quite a lot, so that may indicate something has changed here too.

I add a Tag that belongs to me, then I try tagsearch but I don't see the Tag I just added

Took me a bit to understand but I get it now. Yes then the user will have to change the selected user. Can be avoided by staying on the "All members" view, but this is still a downside.

Also when I browse to that user's Dataset, I can filter by my Tag (above the thumbnails panel).

I feel this is a different feature, to find images inside dataset, also restricting the choice of available tags. It doesn't help to find data in the group scope. So I would argue the two approach are complementary.

The reason I introduced it is because it solves something annoying I've seen with identical tag name. I feels like plugging a USB key, picking the wrong side first (also works with 3+ tags). So I have to pick the second, then remove the first.

Now, when a user takes care of his tags, it depends less on what the other users of the group are doing.

@will-moore
Copy link
Contributor

I'm afraid I missed the tooltip. But the text you suggest sounds fine.

Just comparing the UI for browsing Tags (or Projects, Screens etc) with the Tag-Search UI:
The former has the user's name at the root of the tree as an extra clue that you are only seeing their data (Users don't need to rely only on the Group / User chooser above, which they may not have used to switch Group/User if they've e.g. just followed a link to user's data):

Screenshot 2024-04-24 at 12 02 10

Screenshot 2024-04-24 at 12 02 29

Is there something equivalent that could be added to TagSearch? I appreciate it's a bit tricky since there's not an equivalent "tree root" place where that would belong, so it's up to you.

@Tom-TBT
Copy link
Member Author

Tom-TBT commented Apr 24, 2024

Maybe the next best thing here is to add the user name next to the title "TAG SEARCH":
TAG SEARCH - User One

Or better, I can add the user icon with the user name (just like for the tree) bellow the TAG SEARCH title, like that (it's a montage):
image

@will-moore
Copy link
Contributor

Yes, that looks good 👍

@Tom-TBT
Copy link
Member Author

Tom-TBT commented Apr 25, 2024

Hi @will-moore,
if there's no further issue, I will go ahead and merge it for a release.

@Tom-TBT Tom-TBT merged commit 03d5654 into master Apr 25, 2024
1 check passed
@Tom-TBT Tom-TBT deleted the exclude_tag branch June 10, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants