Prettify the notifications menu #18351

hishamco · 2025-09-13T05:33:20Z

This PR prettifies the notifications menu as follows:

Maximize the notification pop-up width to display the titles properly
Fixes the margin, padding, and font sizes
Remove unread behavior while the mouse is hovering, coz it isn't very pleasant
Use title instead of summary

Before

With notifications

Without notifications

After

With notifications

Without notifications

hishamco · 2025-09-14T19:07:22Z

@Piedone, could you please review if you have time?

Piedone · 2025-09-14T23:41:29Z

@MikeAlhayek is better suited to review this.

Why did you remove initialize from the JS, BTW?

hishamco · 2025-09-14T23:43:32Z

Why did you remove initialize from the JS, BTW?

Basically, it's used for initializing the item removal from the notifications pop-up, which is annoying, as I mentioned earlier

MikeAlhayek · 2025-09-15T00:36:45Z

How does it look on mobile and how does it look when you have many notifications?

MikeAlhayek · 2025-09-15T00:38:09Z

src/OrchardCore.Modules/OrchardCore.Notifications/Views/UserNotificationNavbar.cshtml


    .notification-navbar {
-        max-width: 20rem;
+        min-width: 300px;


Why px instead of rem?

Why "rem" while others are in "px?

rem is the correct approach as it depends on the font size while px is fixed. Please only make changes if you are improving things. In this case, your not

MikeAlhayek · 2025-09-15T00:40:10Z

src/OrchardCore.Modules/OrchardCore.Notifications/Assets/js/notification-manager.js

        return false;
    }
-
-    const initialize = (readUrl, unreadBadgeSelector, wrapperSelector) => {


Please don't remove this behavior. This behavior marks the notification as read as you hover over it and will reduce the total of the unread messages. I am okay with updating the css style, but this is a required feature and should be kept

This is only happening in the pop-up, not in the list page. Also, when you hover on the notification accidentally, it marks it as read. Don't forgot that the subject is displayed, so it's not enough to mark the notification is read why the actual notificationfication is read; we need to remove the subject & summary and replace them with a message property

Someone could try it to see what I mean.

Not how this is intended to work. Most notifications and short and the subject is all you need. Having to click on every notification to mark it read is extremely annoying. If we don't want the hover behavior, then we'll need to add a button next to each notification for the user to click on to mark it as read via Ajax call "without redirecting or closing the menu".

Also, the way it's setup today, one can create a notification using HTML and make the notification clickable. For example "Article,1 was being updated" and "Article 1" would be a link to view it.

Not how this is intended to work. Most notifications and short and the subject is all you need

So, no need for the summary anymore, we might also limit how many notifications show on the list

If we don't want the hover behavior, then we'll need to add a button next to each notification for the user to click on to mark it as read via Ajax call "without redirecting or closing the menu".

Make sense.

Limit notifications in the list
This is already supported: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Notifications/Drivers/NotificationNavbarDisplayDriver.cs#L48

We also use caching to avoid querying the database on every page load.

Remove the summary

The summary is still needed, as it represents the body of the notification:

OrchardCore/src/OrchardCore.Modules/OrchardCore.Notifications/Views/Notification.Header.cshtml

Line 23 in 34ae4d8

@Html.Raw(notification.Summary)

The subject is displayed in the Admin List alongside the summary:

OrchardCore/src/OrchardCore.Modules/OrchardCore.Notifications/Views/Notification.SummaryAdmin.cshtml

Lines 36 to 40 in 34ae4d8

@notification.Subject



@Html.Raw(notification.Summary)



Therefore, both Subject and Summary should be kept.

Additionally, the user must be able to mark notifications as read directly from the menu (via mouseover or a separate button).

If the behavior changed to button, one use-case to solve: if the notification body contains clickable custom HTML, clicking that link should also automatically mark the notification as read. It wouldn’t make sense for the user to click the link but leave the notification in an unread state.

Also, marking the notification as read should automatically update the total unread messaged.

The summary is still needed, as it represents the body of the notification:

It showed up in the pop-up, which doesn't make sense to me. We should use the subject instead

Additionally, the user must be able to mark notifications as read directly from the menu (via mouseover or a separate button).

I agree if there's a button to make it read or remove it from the pop-up

It showed up in the pop-up, which doesn't make sense to me

Subject does not support HTML while Summary does. The Subject or Summary can be generated using Liquid in the workflow as well which is very flexible.

I'm not sure why this takes so long. I don't have any issue with where it was generated, because it could be called from user code. My simple question is if both the subject & summary (body) are required, we need to display the subject in the pop-up like a usual email, then if the user is interested, he could check the summary, which supports HTML

I'm not sure why this takes so long.

I am not sure what you are referring to.

My simple question is if both the subject & summary (body) are required, we need to display the subject in the pop-up like a usual email,

Because the idea behind the notification menu to show the a smaller version of the notification. In other words the body of the notification (summary) not the subject. The subject is intended for the Admin list. If in your own code you want to show the Subject instead, you can override the Notification.Header.cshtm view and show the Subject instead of summary. Showing the summary provides flexibility because of the HTML support where the subject does not.

It's not a matter of my code; from the user's perspective, even in GitHub or any other application, it shows the subject, not the body. Moreover, the summary contains HTML, which might make the pop-up look ugly

MikeAlhayek · 2025-09-15T00:41:48Z

src/OrchardCore/OrchardCore.Notifications.Core/Models/NotificationMessage.cs

 public class NotificationMessage : INotificationMessage
 {
-    public string Subject { get; set; }
+    public required string Subject { get; set; }


I don't recall Subject was required. I could be wrong since it been a while. But why do you think it should be required?

If you send a notification with an empty summary, which I did, I saw that the entire notifications pop-up is broken

Empty summary? But here you are requiring a subject

Yep, to make it the only thing needed for the notification

Subject should not be required in this case.

The quickest way is to leave the behavior as-is

Believe it or not, I was trying to do things right, even if it has been used on many sites. I remembered I'm still suffering from updating my 4 email modules after your proposal :)

So let's do the right thing

So let's do the right thing

If you noticed, I have not been leaving reviews or getting involve into PR because I don't have time to spare on debates like this one. If you have a proposal, I suggest opening up and issue for triage to discuss the changes and determine if there is a value in the change. Meanwhile leave this PR as is and we'll see what the discussion in triage leads. Another approach, do what every thing think is right, and demo the changes in Tuesday meeting this way we can discuss the proposal after a demo

If you noticed, I have not been leaving reviews or getting involve into PR because I don't have time to spare on debates like this one

I'm not sure why this is happening almost every time I propose a PR that changes something that you made, you insist on leaving it as it is.

That's why I pinged other guys, coz I know we ended up with unnecessary debates

FYI, I mentioned what the PR is fixing in the description. I will leave @sebastienros to decide during the triage meeting

Marking the notifications as read on hover would be abnormal. Are there any guidelines from the designer, on the behaviour of the notifications? Otherwise, why can't it mimic the behaviour of another popular or familiar site, let's say GitHub or Facebook, or some other site that has actually put some effort on the design, rather than have it subjective to the programmer?

I'm not sure why this is happening almost every time I propose a PR that changes something that you made, you insist on leaving it as it is.

I am sorry you feel that way. To clarify, I’m not insisting on keeping things exactly as they are, but I also don’t want us to lose existing functionality. Part of the original design requirement was to allow HTML-based notifications, where some could be clickable.

I’m fine with adjusting the mouseover behavior. In fact, I suggested earlier that we could add a “mark as read” button if you feel that would be a better approach.

The idea behind notifications is to support two scenarios:

Text-based notifications: simple messages that only display text.

HTML-based notifications: richer messages that can be interactive, such as linking directly to a content item or external resource.

hishamco · 2025-09-15T05:32:00Z

How does it look on mobile and how does it look when you have many notifications?

I didn't change how it looks on mobile, but I will take a screenshot for both cases

hishamco · 2025-09-16T22:01:03Z

This is how it originally looks when a summary is not supplied

This is how it originally looks when an HTML summary is used accidentally

That's why I suggest showing the subject instead

MikeAlhayek · 2025-09-17T03:05:46Z

@hishamco what if you make Summary required then? Or, on the render logic if Sunnary does not have a value, fallback to the Summary. But this may make the admin center look funky.

hishamco · 2025-09-17T06:23:10Z

That's why I'm suggesting using the subject instead, like other apps, I could leave this to a triage meeting

github-actions · 2025-11-16T16:42:15Z

It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.

hishamco · 2025-11-25T16:33:52Z

@sebastienros, any chance to revisit this in the upcoming triag meeting

sebastienros · 2025-12-11T19:26:13Z

Can you share a video showing how the change mike mentioned is affected?

hishamco · 2025-12-11T21:33:10Z

Which is one @sebastienros?

sebastienros · 2026-01-08T18:50:53Z

How does it look on mobile and how does it look when you have many notifications?

hishamco · 2026-01-08T18:53:58Z

I will attach a screenshot, but it's much better than the previous one

MikeAlhayek · 2026-01-09T04:31:34Z

@sebastienros one important thing here is to NOT lost capabilities. I think last time I reviewed this PR, it prevent us from creating HTML based notifications which we can't lose.

@hishamco did you address my concerns about not being able to use HTML for notifications? Like adding a link or clickable notification?

hishamco · 2026-01-10T10:55:56Z

Notifications.mp4

hishamco · 2026-01-10T11:02:51Z

@sebastienros one important thing here is to NOT lost capabilities. I think last time I reviewed this PR, it prevent us from creating HTML based notifications which we can't lose.

We didn't lose the HTML-based notifications; we render the subject in the notification to prevent the UI from being distorted

TBH, this PR takes too long to merge for a simple UI improvement :)

hishamco · 2026-01-23T13:51:22Z

Can we take an action about this?

gvkries · 2026-01-23T14:13:43Z

The UI changes seem to be warranted IMHO, but as far as I understand the blockage here, please don't remove existing functionality.

hishamco · 2026-01-23T15:04:19Z

please don't remove existing functionality.

Which do you mean? If you refer to hover changes, please try it before then you will realize it's weird

gvkries · 2026-01-23T16:23:01Z

Which do you mean? If you refer to hover changes, please try it before then you will realize it's weird

That may be true, but is not on me to decide. I would say if you want to change that, provide an alternative for the existing functionality, e.g. the discussed button.

hishamco · 2026-01-24T18:43:36Z

This might be true if we have sort of notification center such GitHub, but for a title and one line summray that's enough IMHO

sebastienros · 2026-01-29T19:09:33Z

We didn't lose the HTML-based notifications; we render the subject in the notification to prevent the UI from being distorted

Then you lose the custom HTML, what's the point of custom HTML if you don't render it? Do you think we should remove this feature?

hishamco · 2026-01-29T20:52:54Z

HTML could be useful if we need to render it somewhere or send it via email. I would rather keep things simple in our notification center, something similar to GitHub, without introducing UI/UX bugs

hishamco added 2 commits September 13, 2025 08:26

Prettify the notifications menu

8118baf

npm build

0f8435f

hishamco requested a review from Skrypt September 13, 2025 05:39

MikeAlhayek requested changes Sep 15, 2025

View reviewed changes

hishamco added the ui/ux label Sep 16, 2025

MikeAlhayek added the needs triage label Sep 17, 2025

github-actions bot added the stale label Nov 16, 2025

github-actions bot removed the stale label Nov 25, 2025

Merge branch 'main' into hishamco/notification-dropdown

a30ec27

Fix the styles in mobile view

50ef983

hishamco requested a review from sebastienros January 10, 2026 11:03

	<p class="m-0 f6 flex-auto @(readInfo.IsRead ? string.Empty : "fw-bold")">@notification.Subject</p>

	<p class="m-0 text-normal pr-2 pr-md-0">
	<small>@Html.Raw(notification.Summary)</small>
	</p>

Prettify the notifications menu #18351

Are you sure you want to change the base?

Prettify the notifications menu #18351

Uh oh!

Conversation

hishamco commented Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hishamco commented Sep 14, 2025

Uh oh!

Piedone commented Sep 14, 2025

Uh oh!

hishamco commented Sep 14, 2025

Uh oh!

MikeAlhayek commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hishamco Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hishamco commented Sep 15, 2025

Uh oh!

hishamco commented Sep 16, 2025

Uh oh!

MikeAlhayek commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hishamco commented Sep 17, 2025

Uh oh!

github-actions bot commented Nov 16, 2025

Uh oh!

hishamco commented Nov 25, 2025

Uh oh!

sebastienros commented Dec 11, 2025

Uh oh!

hishamco commented Dec 11, 2025

hishamco commented Sep 13, 2025 •

edited

Loading

hishamco Sep 15, 2025 •

edited

Loading

MikeAlhayek commented Sep 17, 2025 •

edited

Loading

hishamco commented Jan 29, 2026 •

edited

Loading