ErikReider / SwayNotificationCenter

A simple GTK based notification daemon for SwayWM
GNU General Public License v3.0
1.3k stars 62 forks source link

Add `"notification-window-height"` #174

Open NotAShelf opened 1 year ago

NotAShelf commented 1 year ago

I think the title is self-explanatory. Found it funny how an option to configure width exists, but height doesn't.

NotAShelf commented 1 year ago

Unless I am missing this config option entirely, it would be pretty much appreciated if this could be added.

NotAShelf commented 1 year ago

Update: I've just noticed your notice in the README, apologies if this comes out as pushy. That was not my intention.

I've decided to take the matters into my hand and attempted adding in the feature myself, but after editing the source code and building manually, the height option is ignored. Which leads me to believe this is a technical limitation. Unless, of course, I am missing something. Which is likely.

I've added

config.json.in

  "notification-window-height": 350,

configSchema.json

 "notification-window-height": {
      "type": "integer",
      "description": "Height of the notification in pixels",
      "default": 350
    },

configModel.vala

     /**
         * Notification window's height, in pixels.
         */
        public int notification_window_height { get; set; default = 350; }

and finally

notificationWindow.vala

this.default_height = ConfigModel.instance.notification_window_height;
NotAShelf commented 1 year ago

I did my best to implement it the same way as width parameter was implemented, but can't find anywhere else that width was referenced. Am I missing anything specific?

ErikReider commented 1 year ago

No worries! :)

If I remember correctly, there should be a constant called MAX_HEIGHT that's used

NotAShelf commented 1 year ago

Hm, yes there is a MAX_HEIGHT constant in src/notificationWindow/notificationWindow.vala. I'll do further testing later today.

NotAShelf commented 1 year ago

Unfortunately I was unable to get a working prototype. I'm not giving up yet, but probably won't be able to implement what I want anytime soon. Feel free to close this issue.

ErikReider commented 1 year ago

Hmm. I have a faint memory of trying to implement this earlier but decided not to. I wonder why... The PR was #38

I'll leave this open for the time being