Could the job-details-pane reset the scroll position to the top when switching between jobs?



3 years ago
3 years ago


(Reporter: KWierso, Assigned: jfrench)




(1 attachment)

47 bytes, text/x-github-pull-request
: review+
Details | Review | Splinter Review


3 years ago
As it stands, if I select a job and then scroll down in the job details pane, then select a different job, the job details pane will still be scrolled down to where it was for the previous job.

I feel it'd be more "right" for it to reset the scroll position to the top of the job details pane when I move between jobs.

Comment 1

3 years ago
I think I have a fix locally. Wes, do you think the info-panel instead/also should be a slightly greater percentage of the overall page height so users don't get a scroll bar to begin with?

Or are you re-sizing the info-panel divider downwards yourself for your own system, which causes the scroll bar more frequently?

I think your proposed solution makes sense, I just thought I'd ask about the source of the problem itself, which may be typical content overflowing the panel. Though granted perhaps with certain job signatures/filters.. they are unpredictably long.
Flags: needinfo?(wkocher)

Comment 2

3 years ago
I'm using the default sizes for the bottom panel in most cases. It does lose some vertical height when the pinboard is open, though. (It's also more of an issue on my Surface where screen space is more limited.)

Most of the time, though, it's because one or more of:
a) the failure lines are really long, spanning multiple lines
b) there are a lot of failure lines displayed
c) the failure lines each have one or more suggested bugs listed

I'd prefer the symptom to be fixed for this one, rather than the cause, though. It feels like selecting a new job resets the entirety of the info-panel, so scroll position should also be reset to the top when a new job is selected.
Flags: needinfo?(wkocher)
Agreed: you *cannot* make it so there is never a scrollbar for the failure summary, we have an infinite capacity for creating failure, and while I can invent a scenario where you would want scroll position persisted, something like going through several failed runs to make sure that each of them ended after a ton of failures with "Output threshold exceeded...", of the literally tens of thousands of times I have scrolled a failure summary, I have wanted that scroll position persisted exactly never times.

Comment 4

3 years ago
Ha, yup sounds about my thinking also. :) PR coming up.
Assignee: nobody → tojonmz
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All

Comment 5

3 years ago
Created attachment 8697914 [details] [review]

Holiday PR for review.
Attachment #8697914 - Flags: review?(cdawson)
Comment on attachment 8697914 [details] [review]

Looks great!  Thanks!
Attachment #8697914 - Flags: review?(cdawson) → review+

Comment 8

3 years ago
Marking fixed per above merge. Will verify on the next push to stage/prod.
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 9

3 years ago
Verified fixed on stage.
You need to log in before you can comment on or make changes to this bug.