automatically hide undo notification 3 seconds after it has been shown
ClosedPublic

Authored by mgallien on Apr 18 2019, 3:41 PM.

Details

Summary

A timer is used to automatically hide the notification 3 seconds after it is being shown.

The duration is currently not user configurable.

BUG: 406603

Test Plan

3 seconds after it is being shown, the notification is automatically removed

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgallien requested review of this revision.Apr 18 2019, 3:41 PM
mgallien created this revision.

Thanks for this!

I think 3 seconds might be a bit too fast. I can imagine non-tech-users barely even noticing it during that time. I would recommend 5 or 6 seconds.

mgallien updated this revision to Diff 56784.Apr 22 2019, 9:21 PM
  • change the hide delay to 6 seconds
pino added a subscriber: pino.Apr 22 2019, 9:53 PM

I think 3 seconds might be a bit too fast. I can imagine non-tech-users barely even noticing it during that time. I would recommend 5 or 6 seconds.

An hardcoded value can still be too short, especially when the notification message is translated (usually translations are longer than English texts...).
What about making it dependent on the length of the text itself?

In D20670#454559, @pino wrote:

I think 3 seconds might be a bit too fast. I can imagine non-tech-users barely even noticing it during that time. I would recommend 5 or 6 seconds.

An hardcoded value can still be too short, especially when the notification message is translated (usually translations are longer than English texts...).
What about making it dependent on the length of the text itself?

The notification system in Plasma currently does this. In practice... it doesn't work. Calculating how long it takes to read something isn't really possible; because everyone reads at a different pace, and some people will notice the message sooner than other people, it's just a guess, same as a hardcoded value. However there's the additional irritation that the timeout changes over time.

If we think that 6 seconds might still too short for some languages, I would recommend increasing it to 7 or 8 instead of trying to be clever. :) But I think there's a good chance 6 will be fine.

I am a bit lost about the best solution.
Do you think it could make sense to let translators have a way to tweak the delay (including a minimum duration enforced by Elisa) ?

I think the simplest approach is to just make sure the duration covers the worst case scenario for languages like Brazilian Portuguese and German that can be very wordy. If it sticks around for a second or two longer than necessary for English, I don't think that's really a problem.

I think 6 seconds is probably long enough already, but if we think it isn't, then let's increase the timeout to 7 seconds and then ship it. :)

mgallien updated this revision to Diff 56943.Apr 25 2019, 7:51 AM
  • change the hide delay to 6 seconds
  • set the hide delay to 7 seconds

rebase on top of 0.4 and sets the hide delay to 7 seconds

ngraham accepted this revision.Apr 25 2019, 1:05 PM

+1, shipit!

This revision is now accepted and ready to land.Apr 25 2019, 1:05 PM
shubham added inline comments.
src/qml/MediaPlayListView.qml
262

Please remove this line and the extra line below

mgallien added inline comments.Apr 25 2019, 3:08 PM
src/qml/MediaPlayListView.qml
262

Is it really easier to read? The readability is an important quality criteria.

shubham added inline comments.Apr 25 2019, 3:22 PM
src/qml/MediaPlayListView.qml
262

That's how the rest file reads ; )

@shubham, @mgallien is the maintainer and author of this code, so I think it's safe to say he's the expert on this. :)

This revision was automatically updated to reflect the committed changes.