- 
                Notifications
    
You must be signed in to change notification settings  - Fork 25
 
refactor(CSS): refactor application of colors #507
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
Conversation
| 
           Deploy preview for helix-ui ready! Built with commit 89a643d  | 
    
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.
TODO:
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.
TODOs
a8fcc4d    to
    cc68095      
    Compare
  
    fab687d    to
    79b5aa9      
    Compare
  
    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.
Notes to aid in review
| background-color: @gray-50; | ||
| border-radius: 2px; | ||
| border: 2px solid transparent; | ||
| color: @gray-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.
migrated to Light DOM CSS
| } | ||
| 
               | 
          ||
| #hxLink { | ||
| color: @gray-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.
Removed because @gray-900 is the default color for the File Tile, so we let inheritance take over.
| &[href]:hover { | ||
| color: @cyan-500; | ||
| &[href] { | ||
| color: var(--hxLink-color, inherit); | 
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.
Using --hxLink-color only when it's an actual link ([href] is present).
| 
               | 
          ||
| :host([invalid]) { | ||
| #hxFileTile { | ||
| border-color: @red-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.
Moved to Light DOM CSS
| // TODO: hook for progress bar fill color | ||
| :host([progress]) { | ||
| #hxLink { | ||
| color: @gray-600; | 
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.
Moved color definition to Light DOM CSS to allow for css inheritance to take over.
| } | ||
| 
               | 
          ||
| // DEPRECATED: remove in v1.0.0 | ||
| hx-modalhead { | 
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.
organized all deprecated styling further below, in one place
| hx-pill { | ||
| background-color: @gray-400; | ||
| border-radius: 1em; | ||
| color: @gray-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.
Migrated from Shadow DOM CSS
| } | ||
| 
               | 
          ||
| > hx-div { | ||
| --padding-left: @space[md]; | 
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.
Updated in response to renaming hx-div CSS custom props
| &::after { | ||
| border-left-color: @green-500; | ||
| } | ||
| } | 
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.
Migrated from Shadow DOM CSS
| :root { | ||
| --hxSelect__trigger-background: @gray-0; | ||
| --hxSelect__trigger-background--disabled: @gray-100; | ||
| } | 
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.
Removed defaults, because default values are now defined inline with the hook definitions.
See src/helix-ui/elements/HXSelectElement.less.
73dc0e1    to
    89a643d      
    Compare
  
    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.
Dev LGTM
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.
Great job! 🎉
Zoom LGTM
JIRA: SURF-1700
LGTM's