Skip to content

filter rating#4694

Merged
jburel merged 13 commits intoome:developfrom
will-moore:filter_rating
Mar 4, 2017
Merged

filter rating#4694
jburel merged 13 commits intoome:developfrom
will-moore:filter_rating

Conversation

@will-moore
Copy link
Copy Markdown
Member

@will-moore will-moore commented Jun 3, 2016

Simple filter by rating functionality for webclient centre panel (as requested by Ilan).

To test,

  • Rate images 1-5. Also log in as another user and rate some images.
  • Add Filters using chooser (rating or name)
  • Rating - click on stars.
  • Rating matches exact number, any user (E.g. 3 stars matches any image that has been rated 3 by any user)
  • Filter by text works as before (filters for any text on each row in table).
  • X is shown on hover and clicking this will hide and clear filter
  • Try adding and clearing filters in various combinations
  • If a range of images are selected before filtering - filtered images will be de-selected.

screen shot 2016-06-08 at 23 33 38

@will-moore will-moore changed the title Basic filter by rating functionality filter rating Jun 3, 2016
@sbesson sbesson added the develop label Jun 5, 2016
@jburel
Copy link
Copy Markdown
Member

jburel commented Jun 6, 2016

Before adding new element, we probably need to review the "filtering" section so clients are in synch
Insight offers several options that need to be considered here too
cc @gusferguson

@will-moore
Copy link
Copy Markdown
Member Author

@jburel That is certainly true, but the reason we have held off adding further filtering here is because of an all-or-nothing approach. Since we don't have time to do ALL, it means that users don't get any improvements. Now that we have a clear message that filtering by rating would be useful, I think it makes sense to add this.

@jburel
Copy link
Copy Markdown
Member

jburel commented Jun 6, 2016

True but we need to be sure that we introduce it in a way that we can expand so that when we can do "all" there is a minimal change/impact on users.

Caching breaks the 'rate, filter, rate, filter' workflow and query is fast enough
to go unnoticed
@gusferguson
Copy link
Copy Markdown

@jburel - last time we tried to improve the menu in Insight we caused more problems than we solved.

@gusferguson
Copy link
Copy Markdown

I will have a look at it.

@jburel
Copy link
Copy Markdown
Member

jburel commented Jun 6, 2016

I am not saying that insight has/is the answer. Several "standard" filtering options will be needed/requested e.g. map annotation usage is increasing and we don't consider it at all.
so we need to see how we can add them progressively w/o trashing/confusing the user every time.

@will-moore
Copy link
Copy Markdown
Member Author

I have added an issue at ome/design#39 with a simple scenario.

@atarkowska
Copy link
Copy Markdown
Member

thanks @will-moore for a quick example. It's definitely is a nice asset to the center panel. We discussed few design issues that I will write down in ome/design#39. One technical issues is that rating call POST, that should be GET really.

}

var setupFiltering = function() {
// All the image filtering functionaliy setup here...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

functionality

@pwalczysko
Copy link
Copy Markdown
Member

It does not seem to work now on cowfish. When Rating is selected, all what happens is that the filtering dropdown menu jumps a bit to the right. (Firefox, Mac)
The workflow is documented in the sequence of screenshots below.

screen shot 2016-06-09 at 10 43 42
screen shot 2016-06-09 at 10 43 49
screen shot 2016-06-09 at 10 43 30

@gusferguson
Copy link
Copy Markdown

@will-moore

I had a different experience in Safari - may help with the debugging..

Selected dataset - select Ratings from Add Filter DD - broken image link appeared in toolbar - see screenshot 1.
Clicked on broken image link and got 3* rating and centre pane went blank - should have as there were 0 images for 3*.
The rating display was however frozen and could not be changed to another * rating.
Dataset was now deselected in data tree - attempting to reselect dataset failed - and nothing showed anything in the centre pane - screenshot 2.
Selecting a different dataset returned centre pane to normal.
Back to original dataset and then ratings filtered worked as expected and filtering was correct for all ratings and the * rating selection in the toolbar worked fine.
Pretty weird!

webclient

webclient

@will-moore
Copy link
Copy Markdown
Member Author

This is failing robot tests https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-merge-robotframework/313/robot/
I'll look at that now...

@gusferguson
Copy link
Copy Markdown

@will-moore

Filter by rating behaves as expected.
Only issue is that Insight now filters by * rating or more whereas Web is just by the rating, not and more - are we going to allow this diversion? cc: @jburel

Note on filter by name: in Safari - the first time it is tried - hitting return clears the centre pane and deselects the dataset - any subsequent search seems to work OK. Could this be related to what was happening yesterday with the ratings filter?

From ratings filter POV - good to merge.

@jburel
Copy link
Copy Markdown
Member

jburel commented Jun 10, 2016

I think we should go over the design issue before merging anything and see how we can "extend" such feature as pointed out by @joshmoore. This will be important for project like IDR

@will-moore
Copy link
Copy Markdown
Member Author

I consider this to be a short-term fix to support users who need filter by rating. We are using the existing filtering strategy, which is kinda broken for bigger datasets since it doesn't support pagination (only filters current page).
Filtering has to be server side to be done properly.
I implemented this in my React.js reworking of center panel (only filtering by name) #4413 and @aleksandra-tarkowska has also tried this for IDR.
Current filtering in the browser is clearly not going to work for IDR etc.
Any extensions of filtering strategy with plugins should happen with this server-side filtering.
But that shouldn't block this PR, which would be of immediate benefit to users.
Happy to discuss...

@pwalczysko
Copy link
Copy Markdown
Member

This works for me now, the bug #4694 (comment) is fixed. I think that the loss of selection in left-hand tree as reported by @gusferguson is caused when you "filter out" the selected image. Dataset selection persists for me as expected. The loss of selection after "filtering out" of the selected image from the central pane is present also on eel latest (not a regression).
I think for the user in question it would be quite important to have the "unrated" and "rated" distinction, just as it is in Insigth though (with this PR you cannot filter out all unrated images as opposed to rated ones, whatever the rating value).

@atarkowska
Copy link
Copy Markdown
Member

@will-moore that is definitely useful feature, and great idea that you hit the server to get results rather then filter page content. Although I think missing bit is that you are not taking any advantage of the server call. I think we should keep center panel in sync with tree otherwise it is not very clear which images were filtered (for example I would like to use context menu after filtering). At the moment it won't also work with pagination.

@atarkowska
Copy link
Copy Markdown
Member

my comment didn't mean to block that PR, overall is an improvement

@will-moore
Copy link
Copy Markdown
Member Author

I went back to my React.js centre panel code (where I have already implemented server-side filtering) and tried to port some of that code to this PR. However, I find myself getting even more bogged down in the updating of the centre panel according to filtering and selection changes in the jsTree (which is what the React.js work solves). See my discussion of this at http://will-moore.github.io/presentations/2016/Web-Planning-Jan-2016/#/8
I would really like to discuss whether the React.js code can go in (to OMERO 5.3.0 or later)?

@will-moore
Copy link
Copy Markdown
Member Author

Decided at today's client meeting that I should present this at next Tuesday's group meeting, alongside the "open with" demo #4630

@jburel
Copy link
Copy Markdown
Member

jburel commented Oct 12, 2016

I think we need to go back to the ldesign issue: point to discuss is customization vs static. Since filter by rating will for example not make sense for IDR, but filter by gene/no gene will (but won't make sense in main stream)

@will-moore
Copy link
Copy Markdown
Member Author

OK, let's discuss. I'm not sure that this feature needs to be customisable initially. Ola has been identifying various places where customisation is needed and adding support for that as required. We may want to do this later with filtering, but I think the limitations of the filtering (browser-based filter what's already in the page) means it's not really useful for IDR?
As discussed above, we should work towards a customisable server-side filtering framework, but this would be a different beast from what's in this PR.

@bramalingam
Copy link
Copy Markdown
Member

Reviewing scenario's for outreach programs where we can only present the web-client : Despite adding ratings, there is no way we can filter images based on rating (in the web-client) . Can we move this PR forward and see if we can bring this functionality into the client at the earliest? (All other annotations are searchable, except for ratings). Ofcourse the ongoing discussion on customisation makes sense, but can we start with the simplest solution and of course add customisation at a later stage?

@joshmoore
Copy link
Copy Markdown
Member

2 cents: IDR's more complex searching/filtering is being covered by https://github.com/ome/omero-mapr for the moment. As long as we come back and unify that with whatever filtering comes here next, I don't foresee any issues.

@will-moore
Copy link
Copy Markdown
Member Author

Can we please discuss what else (if anything) is needed to get this PR merged?
Seems to be general agreement that this feature is important and no reason to block it going in...
Thanks.

@will-moore
Copy link
Copy Markdown
Member Author

Added robot tests in #4990.

$.getJSON("{% url 'api_annotations' %}?type=rating&" + query, function(data){
// map imageId to rating... rdata = {'iid': show?}
var rdata = data.annotations.reduce(function(prev, r){
// if (r.link.owner.id === WEBCLIENT.USER.id) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove commented out code

}
return prev;
}, {});
$("#dataIcons li.row").each(function(){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

space

var setupFiltering = function() {
// All the image filtering functionaliy setup here...
// Chooser for revealing various filter components
$("#choosefilter").change(function(){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

space

@will-moore will-moore closed this Dec 14, 2016
@will-moore will-moore reopened this Dec 14, 2016
@jburel
Copy link
Copy Markdown
Member

jburel commented Dec 15, 2016

In order to merge this PR, it will be good to have the tests in #4990 passing
Could you revert changes made to components/tests/ui/resources/web/tree.txt? so that the gh-4990 can be included in the build

@will-moore
Copy link
Copy Markdown
Member Author

@jburel 22139a7 should fix merge conflict.

@jburel jburel removed the ON_HOLD label Dec 15, 2016
@will-moore
Copy link
Copy Markdown
Member Author

Robot tests in #4990 are passing better now.
Good to merge?

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2017

When I choose the filter "By Rating", Filter with 5 greyed out stars is displayed
as soon as I click on a stars e.g. "2 stars raining" I have no way using this filter to go back to the default situation
I have to either close the filter or select another one,

Also the functionality does not match insight (there is currently a crash that needs to be fixed in insight). It has always been 1 or more for example
in the web it is the value only.

There is no synchronization between right-hand panel e.g. change rating to 4
and the filter in the centre panel. The image was initially rated at 3 and displayed.
So after saving it should be removed from the display
The synchronization is probably out of the scope of this PR
but a more obvious way to refresh the filtering will be useful. I have for example to know to reclick on the 3 stars in order to achieve a refresh

@jburel jburel mentioned this pull request Feb 23, 2017
@will-moore
Copy link
Copy Markdown
Member Author

There isn't a star you can click on to choose 0 stars. To remove all filtering you have to click on the X. This is the same as when we're adding Ratings, so it will be familiar to users.

I originally filtered "by n or more stars" but this is really less useful since you may actually want to find all the images with 3 stars. This way, users can usefully rate 1-5 to represent 5 different categories and filter by each in turn. If we only filter "by n or more" then this can't be done.

Synchronisation is tricky - I think it would be quite disturbing to click on an Image rating and suddenly the image disappears from view because it's being filtered. Also I think it's a bit of an overkill (too much unnecessary clutter) to add a Refresh button since clicking on the rating filter will simply re-filter. I don't think users are going to take a long time to think how to do this.

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 23, 2017

@will-moore: I was not suggesting a refresh button. Maybe a tooltip to indicate to "click on the star again" to reclassify

If I want to go from 0 to 1 then back to 0 then 1 for example, this requires a large number of clicks in the current implementation

  • bring up the filtering, click on 1 star, click on x, go back to the drop down menu, repeat
    That workflow is not great in my opinion. Let's discuss option tomorrow

@will-moore
Copy link
Copy Markdown
Member Author

Now, clicking X on rating filter will reset it to 0 stars (no filtering) instead of removing it, making it easier to toggle filter on/off.
I've removed the X from name filter since you don't need it to undo filtering (simply clear the text) but I've left the X to remove the "Unrated" filter since there was no other way to toggle the "Unrated" filter.
You can remove ALL filters with an extra option under the "Add filter" selector.

To test, try adding, clearing & removing filters and combinations of Name and Rating/Unrated filters.

@jburel
Copy link
Copy Markdown
Member

jburel commented Feb 27, 2017

I think it works better with the latest adjustment
We will certainly need to revisit the filtering later on and/or mechanism to customize it.

@jburel jburel merged commit 9ab8a98 into ome:develop Mar 4, 2017
@jburel jburel added this to the 5.3.0 milestone Mar 29, 2017
@will-moore will-moore deleted the filter_rating branch February 18, 2019 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants