Notification_Server: add the ability to choose the position of notifications (easy)

[Giova84]

Currently, notifications are displayed where the Deskbar is placed, but would be nice have the ability to choose (in Notifications preflet) where to show notifications, maybe in the four corners of the screen. Mockup attached.

[dsjonny]

+1

[hrily]

I would like to contribute to this issue.

If I'm not wrong, I'll need to figure out:

+ How to change the position of the notification

+ Where are the preference values saved, and how to add one

+ How to add new field to the Preferences GUI

[pulkomandy]

Yes, this sounds right.

Pointers to useful parts of the sourcecode:

• ​http://cgit.haiku-os.org/haiku/tree/src/preferences/notifications/GeneralView.cpp (settings UI)
• ​http://cgit.haiku-os.org/haiku/tree/src/servers/notification/NotificationWindow.cpp#n262 (notification window positionning)

[hrily]

Thank you!!! :)

That will save my extra work.

[hrily]

Finally Booted Haiku.

I had few doubts:

+ Should I add the functionality to change the icon size (as seen in the mockup), since it's not currently present?

[pulkomandy]

No, this was removed from the settings since the mockup was created. The small 16x16 icon was not visible enough.

[hrily]

Okay

So should I create extra tab viz "Display", or should I keep the "Position" inside "General" tab?

[hrily]

Also

How can I test the notifications???

[Janus]

Command notify

[hrily]

One more doubt

If the user chooses the same corner as deskbar, should we show the notification over the deskbar or besides it??

[Janus]

In my opinion it should work as it works now if is in the corner of the deskbar...

It would be nice to have the option "follow the deskbar" and keep the actual code, but I don't know what the other think

[pulkomandy]

Yes, the current behavior of "near the deskbar" should be kept and be the default.

And if the user choses the same corner as deskbar, the notification should still be besides deskbar; not over or under it.

You should keep the position in the general tab.

[hrily]

Okay

Thank you!

Seems like interesting problem to solve... :D

[hrily]

Hie

I've implemented following behavior:

+ If user specifies same corner as deskbar, then show notification besides it.

+ If user specifies same position when deskbar is at top or bottom, then show notification below or above the deskbar.

+ If user specifies "Follow Deskbar", then follow original behavior.

+ If user specifies corner other than deskbar, then show notification on that corner.

Please confirm this, I'll send the patch if this is okay...

[pulkomandy]

That sounds right, but we want to review the code and see if you did not miss any edge case (such as what happens if the setting is currently at the same corner as deskbar, but then deskbar is moved elsewhere).

So, please send the patch :)

[hrily]

@PulkoMandy

There wasn't any comment after the patch upload. So I thought I should drop a message.

Did you start reviewing the code?

[pulkomandy]

Sorry, I forgot about it. The logic looks fine, but there are some style problems.

"loc" is not a good name for a variable. Use "location". I would consider reusing the existing B_FOLLOW_* constants instead of introducing new ones?

[hrily]

Style - I used checkstyle.py, the result was fine. Can you specify?

Location - I will.

B_FOLLOW_* - I didn't get this part...

[pulkomandy]

The only style issue (that I can see) is the name of the "loc" variable.

For the B_FOLLOW_* things: we already have B_FOLLOW_LEFT, B_FOLLOW_TOP, etc constants. I think it would make sense to use these instead of creating a new enum with B_NOTIFICATION_LOWER_RIGHT, etc.

So you would use B_FOLLOW_LEFT | B_FOLLOW_BOTTOM for a "lower left" setting, for example.

[hrily]

Really sorry for the delayed reply. My laptop had a little issue the day of last reply.

I'll send a new patch with these changes asap...

One doubt: What should we use for "Follow Deskbar" option. (I think B_FOLLOW_NONE should do...)

[pulkomandy]

Yes, maybe have a #define B_FOLLOW_DESKBAR B_FOLLOW_NONE so we can understand it more easily when reading the code?

[hrily]

Okay

Should I do this for all the settings, of just B_FOLLOW_DESKBAR?

[pulkomandy]

Just B_FOLLOW_DESKBAR, the others already have the right names?

[hrily]

I've added the patch.

Please review...

[pulkomandy]

Applied in hrev51704, however it breaks the 64-bit build: ​https://buildbot.haiku-os.org/builders/haiku-master-x86_64/builds/4196/steps/jam%20%40minimum-raw/logs/stdio

Can you fix it?

[ohnx56]

Looks like it was fixed by waddlesplash already. (hrev51706)

[hrily]

Patch with fixes for 64bit build

[hrily]

Sorry for the trouble.

Was travelling few days so couldn't reply.

I see that it is fixed already, good to see that...

I had a great time solving this.

[pulkomandy]

Closing again as the 64bit fix has already been done.