Refactor/2318/migrate attribute type selector #2519
Conversation
Tests are still missing. ref: #2318
ref: 2318
Thx for the catch! Fixed with f9f2419 |
visualization/app/codeCharta/ui/attributeTypeSelector/attributeTypeSelector.component.scss
Outdated
Show resolved
Hide resolved
visualization/app/codeCharta/ui/attributeTypeSelector/attributeTypeSelector.component.scss
Show resolved
Hide resolved
| left: 8px; | ||
| } | ||
| &:hover { | ||
| background-color: rgb(200, 200, 200); |
There was a problem hiding this comment.
We used #c8c8c8 before, for most hovers. But this is a personal preference to use hex codes over rgb
| </div> | ||
|
|
||
| <mat-menu #menu> | ||
| <div class="menu-header" mat-menu-item disabled>Set global aggregation for rloc</div> |
There was a problem hiding this comment.
Personal preference, but i think a span or any other generic text element would be cleaner than just writing the text straight into a div, since we could in theory have spans standardized across the project but not generic text in divs.
There was a problem hiding this comment.
The advantage of this standard isn't clear to me. Anyway, lets not spend time in something which we might do, while "just" migrating to Angular, which is already a very very huge task :P
There was a problem hiding this comment.
Text just in a div has no semantic meaning for text, its used for layout, while span is used to style texts. If that makes more sense
| } | ||
| } | ||
|
|
||
| .mat-menu-item { |
There was a problem hiding this comment.
.mat-menu-item is a child of .mat-menu-content and could be in the same paranthesis level as .menu-header above
| } | ||
|
|
||
| .mat-menu-item { | ||
| font-size: 16px; |
There was a problem hiding this comment.
Generally id preffer pt over px, when working with fonts
There was a problem hiding this comment.
Generally I agree. But as we mix it up everywhere and have no concept for it so far, I don't want to invest any time into it while migrating.
…eTypeSelector.component.scss Co-authored-by: Christian-Eberhard <42114276+Christian-Eberhard@users.noreply.github.com>
|
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!
|
|
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
"edges"as it was unused.font-size: 15px->font-size: 16pxas it looks better to me. I think not worth any discussion or mention in changelog.