-
Notifications
You must be signed in to change notification settings - Fork 0
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
WEB-15: Implement Announcement Preview Popup #29
base: main
Are you sure you want to change the base?
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.
This looks awesome! Just a couple changes to your grouping approach as well as CSS styling, but other than that, this is very solid.
I collapsed a few div elements in the image below, but overall it should be grouped like such:
Make sure that when you create a commit, please prepend WEB-15:
to the commit message. For example, WEB-15: Fix div element grouping
.
Very impressive work Belle!
- Also addressed PR comments
Overview
Implement UI for tapping on an announcement within Active Announcements or Past Announcements and displaying a popup.
Changes Made
dateInRange
is called within existing code (AnnouncementCell
) because theLiveIndicator
was not being displayed properly. I discovered the existing code was callingdateInRange
with the arguments in the wrong order.lauren.png
)ModalLiveIndicator
,ModalUpcomingIndicator
,ModalPastIndicator.
Implemented a new variant of the existingLiveIndicator
because the existing one was statically programmed to be in a specific position.ModalLiveIndicator
,ModalUpcomingIndicator
, andModalPastIndicator
are all dynamically positioned.ButtonPrimary2
for the "Delete Announcement" buttonButtonPrimary3
for the "End Live Announcement" buttonButtonPrimary2
andButtonPrimary3
because the existingPrimary1
andSecondary1
hardcode the caret icon.AnnouncementModal.tsx
to handle all the UI logic for the popupScreenshots