-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cde create vis app mvp2 #735
Conversation
…into cde-create-vis-app-mvp2
…into cde-create-vis-app-mvp2
…into cde-create-vis-app-mvp2
☁️ Nx Cloud ReportCI is running/has finished running commands for commit b220efc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
🚀 Preview Deploy Report Updated✅ Successfully deployed preview here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use css variables and components from the design system
<mat-icon class="material-symbols-rounded">upload</mat-icon> | ||
Upload CSV | ||
</button> | ||
<button mat-flat-button class="upload" type="button" color="primary" (click)="fileInput.click()">Upload CSV</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to use color="primary"
any longer
@@ -2,7 +2,7 @@ | |||
@use '@angular/material' as mat; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change all uses of mat.get-theme-color to use css variables instead, i.e. var(--sys-primary)
, etc.
@@ -2,7 +2,7 @@ | |||
@use '@angular/material' as mat; | |||
@use '../../../styles/constants/palettes' as palettes; | |||
|
|||
$blue: mat.m2-define-palette(map.get(palettes.$palettes, 'blue'), 600, 800, 900); | |||
$blue: mat.m2-define-palette(map.get(palettes.$palettes, 'blue'), 500, 600, 800, 900); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these palette variables
), | ||
) | ||
); | ||
|
||
:host { | ||
--mdc-filled-button-label-text-size: 1rem; | ||
--mat-filled-button-horizontal-padding: 0.5rem; | ||
--mdc-filled-button-label-text-tracking: 0.02em; | ||
--mat-filled-button-icon-spacing: 0.25rem; | ||
@include mat.button-color($blue-theme); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
margin-bottom: 2rem; | ||
background-color: rgb(from #ffdcc4 r g b / 0.8); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use css variables
@@ -13,8 +13,7 @@ <h1 class="title">Create a Cell Distance Visualization</h1> | |||
<div class="card data-upload"> | |||
<h2 class="header"> | |||
<span class="step-number">1</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use new step indicator component from design-system
@@ -4,6 +4,7 @@ | |||
@use '../../../styles/breakpoints' as breakpoints; | |||
|
|||
$gray-palette: map.get(palettes.$palettes, 'gray'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move all the logic here into the regular scss file and use css variables instead
@LibbyUX Our design system has been implemented for the create vis page, please review and let me know if there are any changes needed! |
…into cde-create-vis-app-mvp2
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thank you so much @edlu77!
I have more work to do on our text field inputs after playing with this preview, but that's a separate issue for the design system. I will expand text fields to have helper text for error states, absolute positioned below the input field.
Thank you for knocking this out, it looks great. Happy Friday!
#579
Various updates to CDE create vis page