feat: notification panel fix

[RadoBoiii]

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: RadoBoiii
:white_check_mark: jelveh
:x: Rishabh Shinde

---

Rishabh Shinde seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[KernelDeimos]

Hi, thanks for opening this. Does this PR replace #1189? I'll do a manual test later today.

[RadoBoiii]

Hi Eric,

Yes, it replaces #1189. I had to sync the repository to the latest version, and my previous commits were discarded.

On Mon, Mar 31, 2025 at 1:12 PM Eric Dubé @.***> wrote:

Hi, thanks for opening this. Does this PR replace #1189 https://github.com/HeyPuter/puter/pull/1189? I'll do a manual test later today.

— Reply to this email directly, view it on GitHub https://github.com/HeyPuter/puter/pull/1240#issuecomment-2766866943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOZQKJDOKHFYFBYKOU5SCWD2XFZQ5AVCNFSM6AAAAAB2DKVFN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRWHA3DMOJUGM . You are receiving this because you authored the thread.Message ID: @.***> [image: KernelDeimos]KernelDeimos left a comment (HeyPuter/puter#1240) https://github.com/HeyPuter/puter/pull/1240#issuecomment-2766866943

Hi, thanks for opening this. Does this PR replace #1189 https://github.com/HeyPuter/puter/pull/1189? I'll do a manual test later today.

— Reply to this email directly, view it on GitHub https://github.com/HeyPuter/puter/pull/1240#issuecomment-2766866943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOZQKJDOKHFYFBYKOU5SCWD2XFZQ5AVCNFSM6AAAAAB2DKVFN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRWHA3DMOJUGM . You are receiving this because you authored the thread.Message ID: @.***>

[RadoBoiii]

Can this be merged to the repository?

[KernelDeimos]

Just adding a note here since we talked outside the repo - this will be merged pending further review from @jelveh

[KernelDeimos]

I just did a manual test. The behavior of displaying notifications seems to be perfect now. I noticed only two issues:

• minor cosmetic issue: contrast of the bell sometimes makes it difficult to see. I'm gonna find out how @jelveh thinks it should look.
• clicking notifications from the panel has different behavior than clicking notification toasts; it's better if it doesn't do anything at all. We can always add functionality later, but we should avoid broken behavior because it decreases the user's confidence in the system.

This is what happens if I click a file share notification from the panel instead of clicking the notification toast; I get this strange broken window:

Manual Test Recording

https://github.com/user-attachments/assets/71fbd41e-27f2-4338-af25-b71fe9f80b98

[RadoBoiii]

Hi, I fixed the issues that you addressed. I am able to see the bell icon perfectly. I don't understand why you are unable to see it. Can you check this PR?

[KernelDeimos]

I just did a quick test:

• I see the issue with clicking notifications has been fixed.
• I noticed style changes in the top bar. These were present before but I hadn't noticed them.

I think what you're trying to do here is simplify the CSS/DOM of the top bar so that we can more easily add icons while ensuring consistency. I agree that this is necessary, but the appearance of the top bar should not change as a result. If the notification panel doesn't depend on these changes, perhaps it's more appropriate as a separate PR.

Padding of icons changed

This is what the padding looks like in this PR:

This is what it looks like on main currently:

The layout in this PR adds more space between icons, causing the icons to take up more space on the top bar.

Profile icon

This is probably related to the padding issue mentioned above. The profile image area gets moved to the top-left of its container, making the UI look broken.

[RadoBoiii]

The notification panel does not depend on these changes. What should I do for the layout? Should I try to fix it or will it be considered as another PR?

[KernelDeimos]

The notification panel does not depend on these changes. What should I do for the layout? Should I try to fix it or will it be considered as another PR?

I'm find with either, but we can likely merge the notification panel sooner if the other changes are moved to a separate PR

[RadoBoiii]

Sure then, let's merge it and I can try to fix it as another PR

[KernelDeimos]

Sure then, let's merge it and I can try to fix it as another PR

What I meant was the CSS changes made in the PR could be moved to another, and that would expedite merging this. I can't merge a PR with CSS regressions on components of the desktop UI.

[RadoBoiii]

Does this look better?

[KernelDeimos]

There are still styles changes. I want all the style changes for the toolbar removed - they don't need to be in this PR. That means comparing with the style on main and making sure this PR doesn't change it. I'll send a patch file shortly that will make the required changes with a git apply.

[KernelDeimos]

Well, I was going to send a .diff or .patch file but, shockingly, GitHub doesn't support these file extensions; so instead, here's a .txt file which really is a diff.

You can apply this with git apply path/to/pr-1240_rm-toolbar-css.diff.txt

pr-1240_rm-toolbar-css.diff.txt

This diff removes the toolbar style changes and makes it consistent with the current style on main. This will make it possible to merge this PR. Right now (prior to applying this diff) the style changes have visual impacts such as making the toolbar icons further apart from each other; this may seem trivial but it's not and it would tie a lot of unrelated concern to this notification panel PR which you wouldn't want to deal with here - definitely better to put those changes in another PR.

[RadoBoiii]

I have run the command and those changes have been applied to the repository

[KernelDeimos]

Perfect, thanks! I'm going to ask @jelveh if he wants to review the UI first but otherwise this looks ready to merge to me

[RadoBoiii]

I think @jelveh has tested it, is it ready to be merged?

[RadoBoiii]

Hey @KernelDeimos can this PR be merged or does it require additional changes

[RadoBoiii]

Any updates on this @KernelDeimos ?

[KernelDeimos]

Any updates on this @KernelDeimos ?

Not yet, we've got a lot on our plates at the moment.