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

Search engine studies #20

Merged
merged 41 commits into from
Feb 1, 2023
Merged

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Oct 3, 2022

Looking to use omero_search_engine backend for searching for studies.

Currently, changes in this PR only affect the search page (not front page).

Auto-complete for "Any" includes Key-Value pairs for Projects and Screens:

Screenshot 2022-10-03 at 16 44 00

@jburel
Copy link
Member

jburel commented Nov 29, 2022

@will-moore code formatting check is red

@jburel
Copy link
Member

jburel commented Nov 29, 2022

The behaviour on idr-testing is very strange.
I am looking for photon in the "search Result page"

Screenshot 2022-11-29 at 13 42 20

Screenshot 2022-11-29 at 13 43 23

Screenshot 2022-11-29 at 13 43 31

@will-moore
Copy link
Member Author

You can't search with key "Any". You need to pick one of the auto-complete options

@jburel
Copy link
Member

jburel commented Dec 6, 2022

@will-moore could you review the formatting? It has been failing with the past few commits

@will-moore will-moore force-pushed the search_engine_studies branch from ce61b1f to 52ed6ab Compare December 7, 2022 14:50
@will-moore
Copy link
Member Author

will-moore commented Dec 8, 2022

TODOs from today's meeting @francesw @jburel @khaledk2 @pwalczysko

  • Always use "contains" operator when redirecting from old "query=..." search pages
  • Use "contains" operator by default
  • When using auto-complete with a specific Attribute (not "Any"), add an "all / contains X" option that will use the currently-entered search term with "contains" query. Choosing other exact autocomplete matches will use "equals".
  • On front page, fix search by Name, don't show "annotations.csv" matches.

@jburel
Copy link
Member

jburel commented Jan 25, 2023

  • Term "experiments+screens" should be used when displaying results
  • Term "undefined" is displayed in from of the screen entries

@@ -743,7 +750,7 @@ class OmeroSearchForm {
let thead = `<li class="studyRow resultsHeader">
<div class="studyColumns">
<div class="caret"></i></div>
<div class="studyId">Study ID</div>
<div title="Experiment or Screen ID" class="studyId">ID</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought we were going for just ID. Let see how it is displayed

@will-moore
Copy link
Member Author

Re-deployed those fixes to idr-testing..

@will-moore
Copy link
Member Author

Before that last commit, we had "experiments" in the first line of auto-complete (when a key is chosen). Now it is experiments/screens:

Screenshot 2023-01-25 at 13 03 18

Also, the results table mentioned "Studies" and "Study ID", but these are now replaced with "Experiments/Projects"

@francesw
Copy link
Member

When searching with "Any, contains, microscopy", the top hit from the pop up is imaging method (50 screens), but when you click on this, the results show 86 experiments/screens.

Screenshot 2023-01-27 at 14 20 46

@will-moore
Copy link
Member Author

will-moore commented Jan 30, 2023

Auto-complete with "Any" key and "contains" now has greater-than/equals sign:
Screenshot 2023-01-30 at 12 38 42

redeployed now

@jburel
Copy link
Member

jburel commented Jan 30, 2023

The symbol > is displayed
But this is not right. The total indicates XXX number of screens/experimenters (Huge number)
but the count is for images. This area really needs to be reviewed, I am leaning towards removing the count from the display for this release and sort it. Even if it is a solely a software release in a week or so

Screenshot 2023-01-30 at 13 44 20

@jburel
Copy link
Member

jburel commented Jan 30, 2023

Screenshot 2023-01-30 at 14 31 53

Should be ``experiment/screen``

@will-moore
Copy link
Member Author

This is my current auto-complete testing matrix:

With "Any" key and "contains":

All auto-complete items use "contains" and "> count". The object types should be correct: "experiments" or "screens" or "experiments/screens" or "images".

Screenshot 2023-01-30 at 20 07 35

With "Any" key and "equals":

Screenshot 2023-01-30 at 20 11 52

Choosing any of these items - the number in the auto-complete should match the search result

With a chosen Key: (Study attribute or Image attribute)

The operator makes no difference to auto-complete. First item returned should be "contains" the current value with correct sum:

Screenshot 2023-01-30 at 20 09 54

Screenshot 2023-01-30 at 20 10 53

@will-moore will-moore force-pushed the search_engine_studies branch from 631d8ae to 5c31a23 Compare January 31, 2023 11:50
@will-moore
Copy link
Member Author

Those last 2 commits address 2 issues discussed this morning (not related to this PR but easier to fix here to avoid conflicts etc):

  • In Safari, the default form submission handling causes the page to be refreshed when you hit enter
  • In all browsers, when some auto-complete results are showing, if you edit the value in the field (either by typing or deleting or both) and then hit Enter before the auto-complete has fetched new results, the previous value will still be highlighted and will be selected to perform a search. Now, we hide the previous autocomplete results as soon as the user types in the field.

@pwalczysko
Copy link

The behaviour is as expected for the two issues fixed in last commit.

I have one more bug:

Have on the search page selected Gene symbol and then typed into Keys Carpenter. I get No results found which is exapected. Then, I click 2 times up arrow and down arrow on my keyboard. The content of the "Value" box is changed to "-1" (note that I did not type in "-1").

Screenshot 2023-01-31 at 13 39 45

Screenshot 2023-01-31 at 13 39 53

@pwalczysko
Copy link

As discussed, I will create a separate issue from #20 (comment)

Ready to merge fmpov

@will-moore
Copy link
Member Author

As discussed just now: @jburel, @francesw Good to merge...

@will-moore will-moore merged commit 0b6d477 into IDR:master Feb 1, 2023
@pwalczysko
Copy link

created issue #24

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.

4 participants