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

Enhanced filtration/sorting for tasks/projects/tasks in projects/cloud storages #4409

Merged
merged 29 commits into from
Mar 11, 2022

Conversation

bsekachev
Copy link
Member

@bsekachev bsekachev commented Mar 2, 2022

Motivation and context

How has this been tested?

Checklist

TODO:

  • Check import task/project features works as expected
  • Fix sorting resets after changing page
  • Fix several resource calls when open a page
  • Fix filters for tasks in projects, they don't work [server side issue]

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2022 Intel Corporation
#
# SPDX-License-Identifier: MIT

@bsekachev bsekachev changed the title [WIP] Enhanced filtration/sorting for tasks/projects/tasks in projects/cloud storages Enhanced filtration/sorting for tasks/projects/tasks in projects/cloud storages Mar 2, 2022
@bsekachev bsekachev requested a review from azhavoro as a code owner March 2, 2022 19:07
Copy link
Contributor

@klakhov klakhov left a comment

Choose a reason for hiding this comment

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

Hey, It's great to see new filtration/sorting panel
I've found several UX issues with filters:

  • When two rules are present "or" doesn't have left border
    image
  • If we have group of rules and ungrouped rule, the line to the left is not straight. Also trash bin in group is a bit too high.
    image
  • Nested groups look too complicated for me, padding on the left seems insufficient to figure out what level the rule is when there are three+ levels. Do we really need such depth?
    image

cvat-ui/src/actions/cloud-storage-actions.ts Outdated Show resolved Hide resolved
@ActiveChooN
Copy link
Contributor

ActiveChooN commented Mar 4, 2022

Great job! Very powerful solution.
Have a few UX comments/questions:

  • It can be helpful to add the ability to create links for search/filter query for sharing between team members like frame URL, or maybe forward filter params to search query as it did in the GitHub search
  • It would be nice to see outside click filters dropdowns hiding
  • Search query is saved between navigation, but filters are not
  • Nested ordering looks very powerful, but are there real cases for that, but not for regular ordering (single parameter + desc/asc)?

Have an issue with models page:
изображение

@nmanovic
Copy link
Contributor

nmanovic commented Mar 4, 2022

@bsekachev , some problems with Models page:

image

@nmanovic
Copy link
Contributor

nmanovic commented Mar 4, 2022

@bsekachev ,

  • It can be helpful to add the ability to create links for search/filter query for sharing between team members like frame URL, or maybe forward filter params to search query as it did in the GitHub search

It will be great if we can see applied filters in URL like https://github.com/openvinotoolkit/cvat/issues?q=is%3Aopen+is%3Aissue+label%3Abug

  • It would be nice to see outside click filters dropdowns hiding

Agree

  • Search query is saved between navigation, but filters are not

If we use correct URLs, the problem will be solved.

ActiveChooN
ActiveChooN previously approved these changes Mar 9, 2022
@nmanovic
Copy link
Contributor

nmanovic commented Mar 9, 2022

@bsekachev , can we show in browser top bar a readable URL like we do in recent filters? It is only for UI, thus we can have here more readable variant for our users. What do you think?

http://localhost:8080/jobs?filter=%7B%22%21%22%3A%7B%22or%22%3A%5B%7B%22%3D%3D%22%3A%5B%7B%22var%22%3A%22state%22%7D%2C%22completed%22%5D%7D%2C%7B%22%3D%3D%22%3A%5B%7B%22var%22%3A%22stage%22%7D%2C%22acceptance%22%5D%7D%5D%7D%7D&page=1

@nmanovic
Copy link
Contributor

nmanovic commented Mar 9, 2022

@bsekachev , if I clear all quick filters and refresh the project/task/job page, my decision isn't saved.

@bsekachev
Copy link
Member Author

@nmanovic

can we show in browser top bar a readable URL like we do in recent filters? It is only for UI, thus we can have here more readable variant for our users. What do you think?

I do not actually think that somebody is going to change this filter via URL. This URL is intended to create a link to a specific request.
What you suggest I think possible, but techically it brings some difficulties, for example we can't decode query string to filter internal representation, 3rdparty parser is necessary and maybe there are other unpredictable issues.

@bsekachev
Copy link
Member Author

@nmanovic

Now URLs are more readable:

image

They can be copied to clipboard and reopened with keeping filter.

About default filters I would suggest followings:

  • Do not apply default filters when open resources page, literally get rid of default filters
  • Remember the latest applied filter in local storage
  • Filter from URL has higher priority than filter from local storage, so if we have two sources when open page, URL filter is restored

@nmanovic
Copy link
Contributor

nmanovic commented Mar 9, 2022

@nmanovic

Now URLs are more readable:

image

They can be copied to clipboard and reopened with keeping filter.

About default filters I would suggest followings:

  • Do not apply default filters when open resources page, literally get rid of default filters
  • Remember the latest applied filter in local storage
  • Filter from URL has higher priority than filter from local storage, so if we have two sources when open page, URL filter is restored

It looks reasonable. I agree with all proposals. The only question I have in my mind, can we unify the format of filters in URL and recent?

@bsekachev
Copy link
Member Author

@nmanovic

I implemented proposals, but it looks like making format in URL the same causes issues, related with restoring filter from such query strings.

We copy it, paste in another tab and there are not guarantees that we can restore filter as it was (because there are more types of operators in json format, and as I said before, react-awesome-query-builder does not provide solution to parse query strings).

In theory we could look into 3rdparty parsers, but again, it does not guarantees as we do not meet issues in the future.

@klakhov
Copy link
Contributor

klakhov commented Mar 10, 2022

@bsekachev
Anyway, what do you think about changing delete button a bit (remove box-shadow and margin-top)?
pQN-L9P3QnQ

@bsekachev
Copy link
Member Author

bsekachev commented Mar 10, 2022

@klakhov

These styles (margin, spaces, etc) are predefined by the library we are using. Maybe it is a good idea to create issue in their repository if you believe the interface looks confusing? https://github.com/ukrbublik/react-awesome-query-builder

@nmanovic
Copy link
Contributor

@bsekachev ,

Problem: a filter is active by it is not applied

  • Build a filter on Jobs page
  • Go to Tasks page
  • Go to Jobs page

image

Possible solution: with the latest implementation it is better to reset filter when you leave a page (see how github issues filter works)

@bsekachev
Copy link
Member Author

@nmanovic

I discussed this issue with Andrey Chernov. He said it is not critical to keep the latest filter if there is an option to setup it in a couple of clicks (quick filters).

@bsekachev
Copy link
Member Author

@nmanovic @ActiveChooN

BTW, do you think it is important to keep sorting when go away from a page and then return back?
If we do not keep filters, it looks a bit inconsistent and GitHub also do not keep sorting in this case.

Sorting and filters are kept if we just update page with F5 for example, because in this case they are read from URL.

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

LGTM

@nmanovic nmanovic merged commit 1225fbb into develop Mar 11, 2022
@nmanovic nmanovic deleted the bs/enhanced_filtering branch March 11, 2022 19:05
@bsekachev
Copy link
Member Author

@TOsmanov Could you please update documentation for all the pages? (Jobs, Tasks, Projects, Tasks in a project, Cloud storages)

@TOsmanov
Copy link
Contributor

@TOsmanov Could you please update documentation for all the pages? (Jobs, Tasks, Projects, Tasks in a project, Cloud storages)

@bsekachev , Yes, I will update the documentation.

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

5 participants