Move notifications bell back in labs#11508
Conversation
germain-gg
added
the
T-Task
weeman1337
left a comment
weeman1337
left a comment
There was a problem hiding this comment.
Tested and works with the new room header. 👍
Should it also work with the existing one for the transitioning phase? Otherwise there would be a labs setting, that doesn't seem to be working.
| "feature_notifications": { | ||
| isFeature: true, | ||
| labsGroup: LabGroup.Messaging, | ||
| displayName: _td("labs|notifications"), |
There was a problem hiding this comment.
I think notifications is a bit too generic here. What about notifications_panel?
There was a problem hiding this comment.
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.
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.
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 |
# 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:
|
6 participants
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.