Closed KevinMulhern closed 1 week ago
Looks great! 🚀
Played around with it a bit, and a few notes:
(Also: out of the scope of this PR, but on the index "Announcements displayed to everyone on the site as a banner." > "Announcements are displayed to everyone on the site as a banner."
Thanks for the feedback @Asartea, great stuff!
It might be nice to be able to actually mark an announcement as expired, for cases where you want it to stop showing
This is a pretty glaring omission when you point it out haha. I agree it would be very nice to have. I think we'd need to switch to a status instead of relying on the expiry date to do it correctly. That edges it ever so slightly out of scope of this. But I'll put a story together for it.
I've gone ahead and put in the rest of your suggestions though. Thanks again!
Steps for QA
Creating a new announcement:
Editing announcement
Deleting announcement
@KevinMulhern I missed the deployed ODE for this. I can test the functionality when it is available again.
Played around with it when the deployment was active, and can confirm all the QA steps checked out
Hey @zachmmeyer, its back up on a review app!
Thank you @KevinMulhern !
✅ Test announcement with working hyperlink.
✅ Announcement is persistent on all pages unless cleared with close button
✅ Text and link change when announcement is edited.
❌ The message box did not like the entire work of Hamlet. Would be cool to see a character limit.
✅ Making an announcement and then deleting it deletes the announcement, no longer displaying it.
I'm not sure how long the deployment is live for but I can set the expiration date to the next day and see if it expires, but that's not part of the AC.
Thanks @zachmmeyer, character limit is a great spot! I've put in a limit of 100 characters - that should hopefully be all we need with the learn more url available.
Review apps are up for 1 day, I can increase that. But I wouldn't worry about QA'ing the expiry, this only adds a new interface for managing announcements to replace our existing one. The code for displaying/hiding expired announcements hasn't changed and been there for years. It's well battle tested 💪
Then it sounds like we're good to go!
Because:
This commit: