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

Work on the Progress Bar UI #247

Merged
merged 4 commits into from
Dec 10, 2021
Merged

Work on the Progress Bar UI #247

merged 4 commits into from
Dec 10, 2021

Conversation

msm-code
Copy link
Contributor

@msm-code msm-code commented Dec 7, 2021

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running mquery, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?
This is my attempt to solve many of the issues plaguing our progress bar, including:
#64
#183
#193

What is the new behaviour?
Unfortunately, I'm very bad at UIs, and these are primarily UI fixes.

  1. I've changed /recent window a bit, to show query duration. I couldn't find a good place for it anywhere, so I've added it below the progress bar:

image

  1. I've also changed how the progress bar works. The main problem (and there were at least two issues about it) is that it's hard to understand what's going on. "Under the hood" ursadb and yara work concurrently, and it's hard to show this on one progress bar.

First I've tried to solve this by adding more information (yay for 5 colors at once on one progress bar), but I saw the error of my ways, repented, and actually removed some information. See the result on the video below:

recording.mp4

The main addition is the spinner in the lower-left corner. It can be in one of the few states:

  • Collecting datasets... - the query execution didn't start. Database doesn't even know how many datasets there are to query.
  • Searching for candidates X/Y (nn%) - mquery is querying ursadb for candidates. Yara matching is done in the background, and is shown on the progress bar (scaled by the percent of queried datasets - for example 50/100 files tested with 1/2 datasets queried = estimated 25% progress).
  • Matching Yara X/Y (nn%) - finally, the final step.

The new behaviour is superior in many ways. For example, when ursadb is slower than mquery it shows progress (more) correctly:

image

Instead of the old behaviour:

image

I'm not sure if it looks good though. But at least I think it's more understandable?

Test plan
Clone this branch and ensure that progress bar works as expected.

Closing issues

fixes #64
fixes #183
fixes #193

@msm-code msm-code added this to the v1.3.0 milestone Dec 8, 2021
@ITAYC0HEN
Copy link
Collaborator

Some notes:

  1. Maybe the "Status: [something]" label is unnecessary because we have enough information to indicate the status (100% is done, not 100% is either processing or new, 0% can be considered "new")
  2. Why do you need the spinner? I mean, the progress bar and spinner are showing that something is in progress

@msm-code
Copy link
Contributor Author

msm-code commented Dec 8, 2021

Why do you need the spinner? I mean, the progress bar and spinner are showing that something is in progress

Sometimes (especially on HDDs and similar slower hardware, and when datasets are large) the db is stuck in the 0/0 state for some time (double digit number of seconds). For me it's obvious that the DB is working, but it may be not obvious for users.

The "spinning" element sends a clear (IMO) message that things are set in motion and that DB is working hard on giving some results.

Even worse, when the users becomes impatient, they may click "query" again, and accidentally create a second similar query. This is another issue (or non-issue, depending on PoV), but clearer UI should solve it.

But I'm not too attached to this particular spinner. Just want to be as clear as possible that the query is running, and not "stuck" when at 0/0.

(Another quick idea - have min-width for progress bar and use animated style like bottom of https://getbootstrap.com/docs/4.0/components/progress/)

@msm-code
Copy link
Contributor Author

msm-code commented Dec 8, 2021

Maybe the "Status: [something]" label is unnecessary because we have enough information to indicate the status (100% is done, not 100% is either processing or new, 0% can be considered "new")

Agree, I think it has become redundant. I'll remove it.

@msm-code
Copy link
Contributor Author

msm-code commented Dec 8, 2021

recording.mp4

Updated version. I've also moved matches to the left, and unified progress bar in the status page with the progess bar in the recents page (they are almost the same now).

@msm-code msm-code merged commit bf2bbae into master Dec 10, 2021
@msm-code msm-code deleted the feature/progress-ui branch December 24, 2021 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants