AndyKilmory
commented
5 months ago This is looking great! A couple of things I've noticed...
It looks like it's possible to crash the app when a date field is malformed or in an unaccepted format in the config. I'm not sure if that's what you want, or if you'd prefer to defend against and omit announcements not in the correct format.
Less dramatic, but the enum fields
categoryandlifespanalso have some interesting effects in the ui if they are misspelled or have a different value.A number of functions are declared in the top level of the React component - ideally it would be great to either cache these functions between re-renders, or move them out of the component as utility functions.
Lastly (and this is more of a product question, than a coding question), it felt unexpected that, when clicking on the 'Please click to acknowledge' error bar, that the bar did not disappear, but some of the other messages do. It feels like the whole bar should be clickable to dismiss (rather than just the cross). It also feels like the other message should not disappear when one message is dismissed. Screen.Recording.2024-04-11.at.17.14.06.mov
The issues regarding incorrect date, category and lifespan issues have been addressed providing reasonable defaults given the expected nature of announcements declared in the application config (ie feature announcements to be acknowledged and then not seen again).
In terms of the banner behaviour - it is not expected that in real world scenarios all 3 lifespan types will be present at one instant. The transient type is designed for simple messaging that we are happy for users to ignore if they want (success messages etc), the session and persistent messages require more user attention and the team deliberately decided to opt for the user having to click the 'x' to acknowledge the message - the choice of the 'x' over other button options was discussed but UX requested the 'x' to be consistent with the IntGEL styling patterns used across BBC Apps. The expectation is that initially only 'announcements' will appear as part of the banner but a future PR could switch other messaging to be sent via the banner and provide a generalised pattern to send messages through the banner.
prout-bot
What does this change do?
This feature adds in a new React component to provide a notification/announcement banner that can display a variety of messages to the user. Initial use case will be to announce the release of new features but it has been designed so that in time it can be used for all user notifications - warnings, error messages, information etc.
The new features are announced by adding items to the announcements array in the application.conf
The addition of patterns to introduce other messages/notifications has been left for a future PR.
If no announcements are included in the application.conf the functionality will have no impact on the application.
The application polls the server one every minute to check for new notifications
The format of the announcement objects is;
[ (array) { (json object) announceId: (string) the unique id of the announcement - should be unique among all active announcements description: (string) the main text to display in the notification notification_banner endDate: (string, optional, format="yyyy-mm-dd") the date beyond which the announcement should not be seen, if not present set as today + 1 year url: (string, optional) a link to a page/document providing further details regarding announcement urlText: (string, optional) text to be included in a-tag hyperlink (will revert to default if not present) category: (string) the type of announcement - will control styling and display, Enum=announcement, information, warning, error, success lifespan: (string) the lifecycle behaviour Enum=transient (message disappears on any click etc), session (message must be acknowledged but action NOT stored in client cookie - used for current session messages) persistent (message must be acknowledged and action stored in client cookie - used for long running announcements) }, ... ]
How should a reviewer test this change?
Add some example notifications to the application.conf and ensure that they displaty correctly and follow the correct lifetime rules (see lifespan options above)
A set of examples could be;
announcements = [ { announceId: "notification_banner", description: "A New Feature Notification Banner", endDate: "2025-03-31", url: "https://runacharainn.com", urlText: "Runach Arainn Glamping", category: "announcement", lifespan: "persistent" }, { announceId: "sort_control", description: "Sort Control", endDate: "2025-04-01", url: "https://www.bbc.co.uk", category: "warning", lifespan: "transient" }, { announceId: "success_ex", description: "This is an example of a success message", endDate: "2025-04-01", category: "success", lifespan: "transient" }, { announceId: "error_msg", description: "This is an example of a error message. Please click to acknowledge.", endDate: "2025-04-01", category: "error", lifespan: "session" }, { announceId: "old_message", description: "This is an old message", endDate: "2024-01-01", category: "success", lifespan: "persistent" } ]
An example of how these notifications should look is;
Tested? Documented?