Have a loading symbol when changing the date range in Perfherder

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: vaibhav1994, Assigned: ShrutiJ)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
When changing a date range (eg: last 7 days -> last 30 days) in perfherder while looking at a graph, perfherder sometimes takes a good amount of time to load it. I don't know if perfherder is actually loading it or there is some kind of problem. It would be good to have a loading symbol like in graph-server, probably beside the drop down menu, to know that perfherder is doing background work for it.
Yeah we could do this. Note that the current series loading code in perfherder is rather inefficient (it loads series one at a time rather than all at once) which might improve things by itself. In general, I'd rather make things fast where possible than add loading indicators. :)
(Reporter)

Comment 2

3 years ago
True, I would like to see improvements for showing results in perfherder! :jmaher told me that there was patch landed to make this faster, lets see if makes the experience better, and then we might not have to add this symbol anymore!

Comment 3

2 years ago
Hi,

I would like to solve this bug. I have implemented the basic part of the bug. A rotating Loading symbol just below the drop down menu . Looks like this : 
http://s32.postimg.org/snvt2ccwl/loading.jpg

It's actually a gif which keeps rotating. Can you suggest any more improvements or changes to this?
Thanks.
Vibhor Sehgal
Hi Vibhor! Sorry I missed this earlier. That looks like a great start. I'd prefer a style more like what we have in the job details panel in treeherder:

https://github.com/mozilla/treeherder/blob/master/ui/css/treeherder-info-panel.css#L126
https://github.com/mozilla/treeherder/blob/master/ui/plugins/similar_jobs/main.html

It probably makes sense to add the CSS to ui/css/treeherder-global.css and import that into ui/perf.html. There might be a bit of extra work involved there to make things look right.

If you're still interested in this bug let me know and I'll assign it to you. :)
Flags: needinfo?(sehgalvibhor)
It looks like vibhor's not around. I'll give it to shruti
Assignee: nobody → shrutijasoria1996
Flags: needinfo?(sehgalvibhor)
For extra bonus points, we could make this an angular component so we don't duplicate this boilerplate over and over:

```html
    <div ng-if="tabs.failureSummary.is_loading" class="overlay">
        <div>
            <span class="fa fa-spinner fa-pulse th-spinner-lg"></span>
        </div>
    </div>
```

If you take that approach, I would put the javascript in `treeherder/ui/js/components/loading.js`
Shruti doesn't have time for this atm, leaving it for someone else to pick up.
Assignee: shrutijasoria1996 → nobody
(Assignee)

Comment 8

2 years ago
wlach, can I take up this bug?
Flags: needinfo?(wlachance)
(In reply to Shruti Jasoria [:ShrutiJ] from comment #8)
> wlach, can I take up this bug?

Of course, feel free to assign it to yourself. You have the permissions! :)
Flags: needinfo?(wlachance)
(Assignee)

Updated

2 years ago
Assignee: nobody → shrutijasoria1996
Created attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master
(Assignee)

Comment 11

2 years ago
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

Hi, I have made an angular component to implement the html script in comment 6. Let me know if anymore changes have to be made.
Attachment #8819773 - Flags: review?(wlachance)
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

This looks pretty good, but could you make it so that the loading panel shows whenever something is loading? This includes the following cases:

1. When coming to the graphs page from a link.
2. When adding a series to the page.

Thanks!
Attachment #8819773 - Flags: review?(wlachance)
(Assignee)

Comment 13

2 years ago
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

I have made a few more changes to include the cases which you've mentioned in comment 12. I hope it's good now.
Attachment #8819773 - Flags: review?(wlachance)
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

Hi Shruti, the code looks good now, however the CSS/HTML needs a bit of tweaking to get the right effect. Could you take another look?
Attachment #8819773 - Flags: review?(wlachance)
(Assignee)

Comment 15

2 years ago
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

I have changed perf.css a bit to get the required loading effect. Let me know if anymore changes have to be made.
Attachment #8819773 - Flags: review?(wlachance)
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

This is looking really good! Just a few more changes requested and then we can land.
Attachment #8819773 - Flags: review?(wlachance) → review-
(Assignee)

Comment 17

2 years ago
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

I have made the changes you had asked for in the PR. I hope it's good to land now.
Attachment #8819773 - Flags: review- → review?(wlachance)

Comment 18

2 years ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/76286eb5bdf2ae77767c45052d6ee7e0115f7138
Bug 1206173 - Have a loading symbol when changing the date range in Perfherder (#2031)
Comment on attachment 8819773 [details] [review]
[treeherder] SJasoria:bug1206173 > mozilla:master

Yup, everything looks good now! Thanks so much.
Attachment #8819773 - Flags: review?(wlachance) → review+
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.