Skip to content
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

Fixed UI Bugs for QMI Wrapped #916

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Fixed UI Bugs for QMI Wrapped #916

merged 4 commits into from
Dec 19, 2024

Conversation

EricWeng23
Copy link
Collaborator

Summary

Since the launch of QMI Wrapped, there were some minor UI bugs that needs to be addressed:

  1. Align the blue top text in the wrapped slides to be consistent with the white texts
  2. Ensure consistency in font size and font weight across the slides
  3. Pin the wrapped notification at the top of the notification banner

Most of the changes in this PR are CSS changes made in Wrapped.scss

Test Plan

Tested via local server

Keep Wrapped Notification on top of the notif banner
Screenshot 2024-12-08 at 6 50 45 PM

Fix font size and weight consistency

Screen.Recording.2024-12-08.at.7.48.40.PM.mov

Notes

Breaking Changes

None

  • I have updated the documentation accordingly.
  • My PR adds a @ts-ignore

@EricWeng23 EricWeng23 requested a review from a team as a code owner December 9, 2024 00:56
Copy link
Contributor

@rgu0114 rgu0114 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes look great!

Copy link
Contributor

@rgu0114 rgu0114 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated w/ changes from this pr that somehow got lost: #901

className="visit bottom-text"
>
<Typography variant="h3">
<div className="visit num-visits"> {wrappedData.officeHourVisits.length} </div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait we're using numVisits instead of officeHourVisits`, I think we might've only deployed this on release branch but master isn't up to date lemme check

@dti-github-bot
Copy link
Member

dti-github-bot commented Dec 19, 2024

[diff-counting] Significant lines: 31.

@rgu0114 rgu0114 merged commit bee32f5 into master Dec 19, 2024
4 checks passed
@rgu0114 rgu0114 deleted the eric/fix_ui branch December 19, 2024 00:48
@rgu0114 rgu0114 mentioned this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants