Skip to content

[Bugfix:Notifications] Settings font alignment#12717

Merged
bmcutler merged 4 commits into
Submitty:mainfrom
jndlansh:bugfix/notification-font-alignment
Apr 6, 2026
Merged

[Bugfix:Notifications] Settings font alignment#12717
bmcutler merged 4 commits into
Submitty:mainfrom
jndlansh:bugfix/notification-font-alignment

Conversation

@jndlansh

@jndlansh jndlansh commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Why is this Change Important & Necessary?

Closes #12716
This change fixes a UI inconsistency where the "Settings" button in the Notifications tab used a different font and was vertically misaligned compared to the "Mark as Seen" and "Show All" buttons.

What is the New Behavior?

The "Settings" button now uses the same font and vertical alignment as the other notification action buttons.
All three buttons appear visually consistent.

Before: "Settings" button text was a different font and 2px higher than the others.
image

After: All buttons have matching font and alignment.
Screenshot 2026-04-02 011531

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Go to the Notifications tab in a course.
  2. Observe the "Settings", "Mark as Seen", and "Show All" buttons.
  3. Confirm that all three buttons now have the same font and are vertically aligned.
  4. Test in both light and dark mode, and on different browsers if possible.

Automated Testing & Documentation

  1. This is a UI-only change; no new unit or end-to-end tests were added.
  2. Existing Cypress UI tests for notifications should continue to pass.
  3. No documentation changes are required.

Other information

  1. Not a breaking change.
  2. No migrations required.
  3. No security concerns.

@github-project-automation github-project-automation Bot moved this to Seeking Reviewer in Submitty Development Apr 1, 2026
@roye2
roye2 self-requested a review April 3, 2026 01:38
@automateprojectmangement automateprojectmangement Bot moved this from Seeking Reviewer to In Review in Submitty Development Apr 3, 2026
@codecov

codecov Bot commented Apr 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.64%. Comparing base (d7b82e3) to head (dd9cfae).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12717   +/-   ##
=========================================
  Coverage     21.64%   21.64%           
  Complexity     9654     9654           
=========================================
  Files           268      268           
  Lines         36244    36244           
  Branches        487      487           
=========================================
  Hits           7845     7845           
  Misses        27916    27916           
  Partials        483      483
Flag Coverage Δ
autograder 21.32% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.66% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

roye2
roye2 requested changes Apr 3, 2026
View reviewed changes

@roye2 roye2 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please see how the Mark as Seen button is written in this file. I believe you should be writing the button similar to that, and then handling building the URL and the onclick in a JS function that you can declare elsewhere in the file (wherever there is space in the above script tag). This would match how other buttons work in Notifications.vue.

image image

@github-project-automation github-project-automation Bot moved this from In Review to Work in Progress in Submitty Development Apr 3, 2026
@jndlansh

jndlansh commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Hey @roye2!
Have made the settings button like the other buttons in NotificationDisplay.vue

Now I have ran the CI checks, only the cypress (gradeable) is failing. Do I have to fix that too?

@roye2

roye2 commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Hey @roye2! Have made the settings button like the other buttons in NotificationDisplay.vue

Now I have ran the CI checks, only the cypress (gradeable) is failing. Do I have to fix that too?

no, that is a flaky test. I will review your changes later today.

@roye2
roye2 self-requested a review April 3, 2026 15:57
@automateprojectmangement automateprojectmangement Bot moved this from Work in Progress to In Review in Submitty Development Apr 3, 2026
roye2
roye2 approved these changes Apr 3, 2026
View reviewed changes

@roye2 roye2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I took a look at the code and the functionality and everything looks good to me! Nice work :)

@github-project-automation github-project-automation Bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Apr 3, 2026
@jndlansh

jndlansh commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

I took a look at the code and the functionality and everything looks good to me! Nice work :)

Thank you @roye2 !

Rkoester47
Rkoester47 approved these changes Apr 3, 2026
View reviewed changes

@Rkoester47 Rkoester47 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a good change. I can confirm that in light/dark mode in both Firefox and Chrome the buttons look uniform and correct now.

John-Roy123
John-Roy123 approved these changes Apr 3, 2026
View reviewed changes

@John-Roy123 John-Roy123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a good UI change and makes the notifications tab look cleaner

@roye2 roye2 moved this from Awaiting Maintainer Review to Ready to Merge in Submitty Development Apr 3, 2026
@roye2 roye2 moved this from Ready to Merge to Awaiting Maintainer Review in Submitty Development Apr 3, 2026
@jndlansh

jndlansh commented Apr 4, 2026

Copy link
Copy Markdown
Contributor Author

Thank you maintainers! @Rkoester47 @John-Roy123 @roye2

@bmcutler
bmcutler merged commit 9e19128 into Submitty:main Apr 6, 2026
48 of 49 checks passed
@github-project-automation github-project-automation Bot moved this from Awaiting Maintainer Review to Done in Submitty Development Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@John-Roy123 John-Roy123 John-Roy123 approved these changes
@Rkoester47 Rkoester47 Rkoester47 approved these changes
@roye2 roye2 roye2 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Submitty Development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Button font and alignment inconsistency

5 participants

@jndlansh @roye2 @John-Roy123 @Rkoester47 @bmcutler