[Status Bar] Remove file status Timer, add text update delay
ClosedPublic

Authored by meven on Nov 8 2019, 3:43 PM.

Details

Summary

Currently when hovering over a file we have its name, mimetype type and size display in the status bar for 1 second, after which the status of the folder is displayed.

This patch removes this timer making the status bar behavior more predictable and user friendly.

Instead there is a 50ms delay between when the status bar gets new text to display (for instance mouse hovering or keyboard navigation) and when the status bar displayed text is updated. This is to avoid flickering.

FIXED-IN: 19.12
BUG: 399267

Diff Detail

Repository
R318 Dolphin
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18620
Build 18638: arc lint + arc unit
meven created this revision.Nov 8 2019, 3:43 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptNov 8 2019, 3:43 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
meven requested review of this revision.Nov 8 2019, 3:43 PM
meven retitled this revision from [Status Bar] Remove file status Timer i to [Status Bar] Remove file status Timer.Nov 8 2019, 3:43 PM
meven updated this revision to Diff 69459.Nov 8 2019, 3:44 PM

Update commit message

Yeah, I see your point. However I think we might still want a timer here, albeit used for a different purpose. Without the timer, when you move the cursor over many files, the status bar display changes repeatedly in rapid succession. The Information panel uses a timer to only update its display when the cursor has been over something for a fraction of a second. Copying that behavior here might make this feel better.

meven updated this revision to Diff 69465.Nov 8 2019, 4:29 PM

Delay by 150ms before the status bar text is updated

ngraham accepted this revision.Nov 8 2019, 4:34 PM
This revision is now accepted and ready to land.Nov 8 2019, 4:34 PM
meven added a comment.Nov 8 2019, 5:53 PM

Will wait for @elvisangelaccio feedback for a few days.

meven added a comment.Nov 9 2019, 7:21 AM

I am not so sure about this 150 ms delay.
When navigating with the keyboard, it is quite noticeable.
So in the meantime, I am gonna reduce this delay to 50 ms.

Removing it altogether might be appropriate.

Just as a reminder, the information panel delay is mostly due to technicalities : thumbnail generation and metadata extraction are costly.
Whereas here we don't have such reasons to limit text refreshing.

meven updated this revision to Diff 69499.Nov 9 2019, 7:24 AM

Reduce update delay to 50ms

I am not so sure about this 150 ms delay.
When navigating with the keyboard, it is quite noticeable.
So in the meantime, I am gonna reduce this delay to 50 ms.

Removing it altogether might be appropriate.

Just as a reminder, the information panel delay is mostly due to technicalities : thumbnail generation and metadata extraction are costly.
Whereas here we don't have such reasons to limit text refreshing.

Another thing to mention is that the information panel is not even visible by default.

This 1s timer was added by 1ea09b24e16d98ac2f1033b without explaining why, so I think we can safely remove it. So I'd restore the previous version of this patch that removed the timer altogether.

meven added a comment.Nov 9 2019, 2:15 PM

I am not so sure about this 150 ms delay.
When navigating with the keyboard, it is quite noticeable.
So in the meantime, I am gonna reduce this delay to 50 ms.

Removing it altogether might be appropriate.

Just as a reminder, the information panel delay is mostly due to technicalities : thumbnail generation and metadata extraction are costly.
Whereas here we don't have such reasons to limit text refreshing.

Another thing to mention is that the information panel is not even visible by default.

This 1s timer was added by 1ea09b24e16d98ac2f1033b without explaining why, so I think we can safely remove it. So I'd restore the previous version of this patch that removed the timer altogether.

I did remove the 1s timer similarly to state before 1ea09b24e16d98ac2f1033b.

But as @ngraham suggested, I added a delay before a new text is displayed in the status bar to avoid flickering similarly to the information panel.
The delay is currently of 50ms.
That is what my last comment was about, not about the 1s timer which I already removed.

Do we understand each other ?
Do you have an opinion about this update delay, taking into account my last comment ?
Or did you mean LGTM ?

I agree that the timer serves no useful purpose for keyboard navigation. But for moving the cursor, I think it's useful to prevent the status bar from updating constantly as you move the cursor over different items and empty space.

elvisangelaccio accepted this revision.Nov 9 2019, 8:45 PM

Do we understand each other ?
Do you have an opinion about this update delay, taking into account my last comment ?
Or did you mean LGTM ?

Sorry if I was not clear. I meant that I'd be ok with no timers at all. However, I also see Nate's point about flickering.

So I'm going to accept the current patch, but please don't forget to amend the commit message before pushing (it should mention the 50ms timer).

meven edited the summary of this revision. (Show Details)Nov 9 2019, 8:50 PM
meven updated this revision to Diff 69508.Nov 9 2019, 8:55 PM

Amend commit comment, add some comments

meven updated this revision to Diff 69509.Nov 9 2019, 8:57 PM

Add m_textTimestamp constructor initializer

meven retitled this revision from [Status Bar] Remove file status Timer to [Status Bar] Remove file status Timer, add text update delay.Nov 9 2019, 8:59 PM
elvisangelaccio accepted this revision.Nov 9 2019, 9:04 PM
This revision was automatically updated to reflect the committed changes.