Skip to content

Fix loading card style inconsistency#1358

Open
SAN-MUYUN wants to merge 3 commits intoCATcher-org:masterfrom
SAN-MUYUN:fix/loading-card-style-mismatch
Open

Fix loading card style inconsistency#1358
SAN-MUYUN wants to merge 3 commits intoCATcher-org:masterfrom
SAN-MUYUN:fix/loading-card-style-mismatch

Conversation

@SAN-MUYUN
Copy link
Copy Markdown

Summary:

Fixes #1330

Changes Made:

Add CSS design to the loading card component, to give a shadow and remove the round edges.

Before:

image is taken from issue #1330 itself
image

After:

image

Proposed Commit Message:

Fix loading card style inconsistency

@damithc damithc requested review from NorbertLoh and Copilot July 7, 2025 06:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the visual inconsistency of the loading card by adding elevation and removing rounded corners.

  • Introduces a .loading-card class on the loading mat-card for consistent styling.
  • Defines CSS rules to flatten corners and maintain shadow depth.
  • Keeps spinner centered with flexbox layout.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/app/shared/issue-tables/issue-tables.component.html Added loading-card class and mat-elevation-z8, removed old tag formatting
src/app/shared/issue-tables/issue-tables.component.css Defined .loading-card styles to override border-radius and add transparent border
Comments suppressed due to low confidence (1)

src/app/shared/issue-tables/issue-tables.component.html:299

  • Angular templates should not reference this; remove the this. prefix so the binding reads *ngIf="issues.isLoading$ | async".
  *ngIf="this.issues.isLoading$ | async"

<mat-card
*ngIf="this.issues.isLoading$ | async"
class="mat-elevation-z8 loading-card"
style="display: flex; justify-content: center; align-items: center"
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider moving inline flexbox styles into the .loading-card CSS class for cleaner templates and centralized styling.

Copilot uses AI. Check for mistakes.
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.

Could consider this if no other components uses loading-card

Comment on lines +83 to +84
.loading-card {
border-radius: 0% !important;
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Avoid using !important if possible; increase selector specificity or adjust style ordering so border-radius: 0; is applied without !important.

Suggested change
.loading-card {
border-radius: 0% !important;
.mat-table .loading-card {
border-radius: 0%;

Copilot uses AI. Check for mistakes.
}

.loading-card {
border-radius: 0% !important;
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Using 0% is functionally equivalent to 0; consider changing to border-radius: 0; for clarity.

Suggested change
border-radius: 0% !important;
border-radius: 0 !important;

Copilot uses AI. Check for mistakes.
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 agree with this

Copy link
Copy Markdown
Contributor

@NorbertLoh NorbertLoh left a comment

Choose a reason for hiding this comment

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

I like the visual change. Since it is mainly a visual change. No logic is touched 👍. Nit to fix otherwise LGTM!

}

.loading-card {
border-radius: 0% !important;
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 agree with this

<mat-card
*ngIf="this.issues.isLoading$ | async"
class="mat-elevation-z8 loading-card"
style="display: flex; justify-content: center; align-items: center"
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.

Could consider this if no other components uses loading-card

<mat-card *ngIf="this.issues.isLoading$ | async" style="display: flex; justify-content: center; align-items: center">
<mat-card
*ngIf="this.issues.isLoading$ | async"
class="mat-elevation-z8 loading-card"
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.

Nitpick. Doing mat-elevation-z8 seems to cast a shadow on the table header. Might be difficult but see if there is any alternative

@SAN-MUYUN
Copy link
Copy Markdown
Author

Thanks for the suggestions! I will look into them and make changes accordingly. Removing the shadow at the top border of the card is a bit more difficult, I will see what I can do about it. 👍

@SAN-MUYUN
Copy link
Copy Markdown
Author

@NorbertLoh The shadow cast on the header table is now removed. Adjustments to code is made according to the suggestions.
image

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.19%. Comparing base (7e5db1a) to head (cfd6528).
⚠️ Report is 15 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
- Coverage   58.39%   58.19%   -0.20%     
==========================================
  Files         104      104              
  Lines        2579     2605      +26     
  Branches      291      306      +15     
==========================================
+ Hits         1506     1516      +10     
- Misses       1022     1038      +16     
  Partials       51       51              

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

🚀 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.

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.

Loading card in table does not match style of table

4 participants