-
Notifications
You must be signed in to change notification settings - Fork 292
Dynamic content (Part 3) #15713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Dynamic content (Part 3) #15713
Conversation
Updated comment for unsetValue property in NotificationPreference type.
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.
Didn't manage to get through this all (specifically components, and testing different values and edge cases). Had some questions on announcements vs notifications vs others again (sorry)
| * - `homepage/rhs` - Shown on the home page as a panel beneath the right-hand side links panel | ||
| * | ||
| */ | ||
| export type Announcement = { |
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.
Probably at the point where we need types for props application to where the announcement is made?
- common
- notification
- home page
| /** | ||
| * Icon information | ||
| */ | ||
| export type AnnouncementNotificationIconData = { |
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.
AnnouncementNotificationData (and its AnnouncementNotificationIconData) aren't used anywhere
|
|
||
| await dispatch('notifications/add', notification); | ||
| } | ||
| await dispatch('notifications/add', notification); |
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.
we've switched to 1:1 announcements to notifications, and notifications are either shown in the header or not with hidden and shown in the home page given a subset of target (location). rather than announcements --> notifications handled by notification process and announcements --> others handled by components + read preference.
it's a bit messier, but it's needed to persist non-notification announcements over refresh?
if so do we still need the announcements read pref, couldn't it just use the notification read state?
Summary
Fixes #15342
Occurred changes and/or fixed issues
This is the last part of the code changes for Dynamic Content for 2.13.
This PR adds support for showing announcements on the Home Page, in addition to in the Notification Centre.
Technical notes summary
The notification store is tweaked slightly, to support a new
Hiddenlevel, which indicates that the notification should not be shown in the notification centre.We add two new getters
hiddenandvisibleto easily get the notifications of these, while leaving theallgetter in place.We also add
datato a notification and ensure this is persisted into the encrypted notification in local storage. This can be used to hold arbitrary data and is used by announcements to hold additional data it needs.The bulk of the logic changes are in
announcement.ts.In addition to notification centre, we support a target of
homepage/*. The*indicates a location in the UI - currently supported arebannerandrhs(right-hand side).For homepage announcements, we also support:
The home page is updated to use two new components:
DynamicContentBannerandDynamicContentPanel.These both use the composable
content.tsto provide access to dynamic content and common code.Both of these panels use common components for the icon/image and the close button.
Note that images use a specific format, which can be:
~IMAGE.svghttps://PATH/IMAGE.svgi.e. we use
~to reference a built-in image. Several images are built-in and added in this PR. We will most likely add additional ones.We also support
!ICONto use a named icon, rather than an image.Additionally, the icon string can include extra info, for example
~IMAGE.svg@64x32#ff0000>12<24this allows the image size, color, padding and margin to be set via the single icon reference.
Areas or cases that should be tested
Notification centre
Home page
Use the dynamic content test UI extension to test
Screenshot/Video
Checklist
Admin,Standard UserandUser Base