-
-
Notifications
You must be signed in to change notification settings - Fork 830
Move notifications bell back in labs #11508
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"feature_notifications": { | ||
isFeature: true, | ||
labsGroup: LabGroup.Messaging, | ||
displayName: _td("labs|notifications"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think notifications
is a bit too generic here. What about notifications_panel
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the current terminology we refer this piece of UI with. I'll defer to @daniellekirkwood for the final wording
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In translation the name is „Notifications panel“. If I see notifications
as a translator I would naively translate it with „Notifications“ / „Benachrichtigungen“ instead of „Notifications panel“ / „Benachrichtigungsbereich“.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Labs flags in product appear to have the structure: Header, subhead so my suggestion would be this:
Enable the notifications panel in the room header
Unreliable in encrypted rooms
Thank you for the review. I have purposefully not written any tests for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks
# Conflicts: # src/i18n/strings/en_EN.json # test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap
@weeman1337 , re-requesting review just to let you know that I'm going to merge this PR even with sonarcloud failing; from #11495:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, works 👍
For element-hq/element-web#25883
Fixes: https://github.com/vector-im/element-internal/issues/444
To review alongside element-hq/element-web#25924
Checklist
This change is marked as an internal change (Task), so will not be included in the changelog.