Skip to content

Plug: rewrite placeholder - #117

Merged
danirabbit merged 1 commit into
mainfrom
danirabbit/plug-rewrite
Aug 25, 2025
Merged

Plug: rewrite placeholder#117
danirabbit merged 1 commit into
mainfrom
danirabbit/plug-rewrite

Conversation

@danirabbit

@danirabbit danirabbit commented Aug 15, 2025

Copy link
Copy Markdown
Member
Screenshot from 2025-08-15 11 38 42
  • Use SPDX copyright header
  • Don't build in multiple single-use functions
  • Remove hard line breaks from placeholder strings
  • Remove useless "nothing to do" title
  • Add a titlebar to the placeholder
  • Check for configurable apps on shown not just when constructed

@danirabbit
danirabbit requested a review from a team August 15, 2025 18:49
@danirabbit

Copy link
Copy Markdown
Member Author

I actually wonder if it's possible to see this screen/if it's worth having this screen since there are entries for settings daemon and "other". Should we just remove this since system settings depends on settings daemon?

@danirabbit danirabbit moved this to Needs review in OS 8.1.0 Aug 15, 2025
@ryonakano

Copy link
Copy Markdown
Member

Should we just remove this since system settings depends on settings daemon?

I personally think we should keep this placeholder even if it would never appear. The backend implementation itself just iterates on installed apps that use notifications and does not add Setting Daemon manually:

https://github.com/elementary/switchboard-plug-notifications/blob/d7f135521c2d24ddb0f431c90ae1ceb280fdfa58/src/Backend/NotifyManager.vala#L31-L48

So I don't think it's guaranteed that Backend.NotifyManager.get_default ().apps is always not a blank HashMap.

since there are entries for settings daemon and "other".

This is a result so I don't think we should do an implementation which assumes that.

@ryonakano

Copy link
Copy Markdown
Member

The issue gone when in case of the main view, but still occurs in case of the placeholder view:

Kooha-2025-08-17-19-19-34.webm
user@elementary-daily:~/work/switchboard-plug-notifications$ git diff
diff --git a/src/Plug.vala b/src/Plug.vala
index e6c75b7..b9c9000 100644
--- a/src/Plug.vala
+++ b/src/Plug.vala
@@ -61,7 +61,8 @@ public class NotificationsPlug : Switchboard.Plug {
     }
 
     public override void shown () {
-        if (Backend.NotifyManager.get_default ().apps.size > 0) {
+        //if (Backend.NotifyManager.get_default ().apps.size > 0) {
+        if (false) {
             stack.visible_child = main_view;
         } else {
             stack.visible_child = placeholder_view;
user@elementary-daily:~/work/switchboard-plug-notifications$ 

@danirabbit

Copy link
Copy Markdown
Member Author

@ryonakano this might be a bug in Granite.Placeholder. I'd still like to get it merged as is since it's an improvement but I've removed the fixes line

@danirabbit
danirabbit merged commit 8ebb9e1 into main Aug 25, 2025
4 checks passed
@danirabbit
danirabbit deleted the danirabbit/plug-rewrite branch August 25, 2025 23:29
@github-project-automation github-project-automation Bot moved this from Needs review to Done in OS 8.1.0 Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants