Skip to content

Conversation

@pmachapman
Copy link
Collaborator

@pmachapman pmachapman commented Nov 19, 2025

This button fixes a regression from #3511 where a style that was supposed to correctly theme an icon button in a notice themed text buttons as well.


This change is Reviewable

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.78%. Comparing base (67c5083) to head (b5e2d7b).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3579   +/-   ##
=======================================
  Coverage   82.78%   82.78%           
=======================================
  Files         608      608           
  Lines       37196    37196           
  Branches     6102     6078   -24     
=======================================
  Hits        30794    30794           
  Misses       5488     5488           
  Partials      914      914           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RaymondLuong3 RaymondLuong3 self-assigned this Nov 20, 2025
Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

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

@RaymondLuong3 reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pmachapman)


src/SIL.XForge.Scripture/ClientApp/src/app/shared/notice/notice.component.scss line 151 at r1 (raw file):

    .mat-mdc-icon-button {
      // use the correct icon button color from the theme
      color: var(--notice-color-text);

I don't see where this is used. Is it necessary?

Code quote:

      color: var(--notice-color-text);

Copy link
Collaborator Author

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)


src/SIL.XForge.Scripture/ClientApp/src/app/shared/notice/notice.component.scss line 151 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

I don't see where this is used. Is it necessary?

It is used on the draft jobs screen for the close icon on the filter notice:

image.png

Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pmachapman)


src/SIL.XForge.Scripture/ClientApp/src/app/shared/notice/notice.component.scss line 151 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

It is used on the draft jobs screen for the close icon on the filter notice:

image.png

Thanks for the explanation.

@RaymondLuong3 RaymondLuong3 merged commit 570ab8a into master Nov 24, 2025
20 of 21 checks passed
@RaymondLuong3 RaymondLuong3 deleted the fix/SF-3642 branch November 24, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants