SED-4893 improve execution report tab-layout - #1444
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces various layout and styling improvements across the dashboard, tabs, and execution progress components, including the addition of visual tab dividers and refined spacing. The review feedback suggests merging duplicate SCSS blocks for better maintainability, replacing an 8-digit hex color with a more readable rgba() function, and refactoring duplicated tab-divider rendering logic in the HTML template.
| .mat-mdc-tab-links { | ||
| min-width: 100%; | ||
| width: max-content; | ||
| overflow: visible; | ||
| } | ||
|
|
||
| .mat-mdc-tab-links { | ||
| align-items: flex-end; | ||
| margin-left: 0; | ||
|
|
||
| > .tab-divider { | ||
| display: block; | ||
| flex: 0 0 0.2rem; | ||
| align-self: center; | ||
| height: 1.6rem; | ||
| margin-inline: -0.1rem; | ||
| background-color: var.$gray-300; | ||
| pointer-events: none; | ||
| } | ||
|
|
||
| > a.mdc-tab--active + .tab-divider, | ||
| > .tab-divider:has(+ a.mdc-tab--active), | ||
| > a:hover + .tab-divider, | ||
| > .tab-divider:has(+ a:hover) { | ||
| display: none; | ||
| } | ||
| } |
There was a problem hiding this comment.
For improved readability and maintainability, you can merge the two separate .mat-mdc-tab-links style blocks into a single one.
.mat-mdc-tab-links {
min-width: 100%;
width: max-content;
overflow: visible;
align-items: flex-end;
margin-left: 0;
> .tab-divider {
display: block;
flex: 0 0 0.2rem;
align-self: center;
height: 1.6rem;
margin-inline: -0.1rem;
background-color: var.$gray-300;
pointer-events: none;
}
> a.mdc-tab--active + .tab-divider,
> .tab-divider:has(+ a.mdc-tab--active),
> a:hover + .tab-divider,
> .tab-divider:has(+ a:hover) {
display: none;
}
}| } | ||
|
|
||
| &:not(.mdc-tab--active):hover { | ||
| background-color: #293e5014; |
There was a problem hiding this comment.
The hardcoded background color #293e5014 uses an 8-digit hex notation which can be less readable and maintainable. Consider using the rgba() function for better clarity, especially since this notation is not as widely supported or understood.
| background-color: #293e5014; | |
| background-color: rgba(41, 62, 80, 0.08); |
| @for (tab of tabs(); track tab.id; let isFirst = $first) { | ||
| @if (!isFirst) { | ||
| <span class="tab-divider" aria-hidden="true"></span> | ||
| } |
There was a problem hiding this comment.
The logic for adding a .tab-divider is duplicated in both branches of the @if (userRouteLinks()) condition (here and in the @else block starting on line 21). This code duplication can make maintenance harder and more error-prone. Consider refactoring to a single @for loop to avoid repeating the tab and divider rendering logic.
1 participant
No description provided.