search

lxqt / lxqt-notificationd

The LXQt notification daemon
https://lxqt.github.io
GNU Lesser General Public License v2.1
62 stars 38 forks source link

Added a preview button, instead of automatic preview #293

Closed tsujan closed 2 years ago

tsujan commented 2 years ago

With it, it's less likely that the preview notification starts at the previous position. With automatic preview, it always started there and then went to the new position (because there's no way to know exactly when the config file is ready).

The preview button can be clicked by pressing Enter too.

Closes https://github.com/lxqt/lxqt-notificationd/issues/236

stefonarch commented 2 years ago

Works fine, but at first install on my main PC which I hardly use recently it looked like not working, even normal notifications. Recompiled master, same, but then I noticed the "only save notifications" checked, with tons of previews saved.

So some explanation maybe is needed below the button like

Note:

If "Do not disturb" is active the preview cannot be shown
stefonarch commented 2 years ago

When changing position during preview it will switch, but only for the default time set, this is actually nice. So maybe make the preview always 15s?

BTW the reset button doesn't reset dimensions.

tsujan commented 2 years ago

When changing position during preview it will switch

Yes. The root of the problem isn't solved by it. But, I think, there are two advantages:

  1. The user isn't in a hurry ;) Usually, he/she doesn't click the preview button immediately.
  2. The preview is also possible when the position isn't changed. The user may just want to see how notifications work in LXQt.

So some explanation maybe is needed below the button like... So maybe make the preview always 15s?

I agree with the first one. But let's limit this PR to its main purpose. We could add separate PRs for other changes later.

BTW the reset button doesn't reset dimensions.

Will check it later (but not in this PR).

tsujan commented 2 years ago

I might also open an issue about the lack of preview in the no-disturb mode. Adding a tooltip or an explanation is one solution but we may be able to make an exception in this case and really show the notification. Will re-read the code later (not for this PR).

stefonarch commented 2 years ago

Ok, step by step. An exception would be the cleanest way.

tsujan commented 2 years ago

An exception would be the cleanest way.

Yes, but I wouldn't cross my fingers because the config code is completely separate from the daemon code.

If it isn't possible, I'll add a popup message (or a red bar inside the GUI) that will be shown on pressing "Preview" with the do-not-disturb mode. It could say something like, "The do-not-disturb mode is enabled. Notifications are saved." After all, it could serve as a demo for the do-not-disturb mode too.

Anyway, I agree that it's a problem to fix. Thanks for finding it!

stefonarch commented 2 years ago

Anyway, I agree that it's a problem to fix. Thanks for finding it!

I didn't find it - it happened casually, usually I never use the "save" mode, it was active for saving this globalkeys issue messages...

tsujan commented 2 years ago

it happened casually

And you found why it happened.

I think I've found a simple way of making an exception for test notifications in the do-not-disturb mode. Will make a PR.

stefonarch commented 2 years ago

I see now that repeated clicking "preview" without changing position opens one below (or upon) another, respecting the "Spacing" settings. Not a big issue.

tsujan commented 2 years ago

Not a big issue.

There's no issue. It really shows how multiple notifications are shown.

stefonarch commented 2 years ago

Oh yes. My brain is older than me in the evening.