[Project Solar / Phase 1 / Components] AppHeader carbonization#3819
[Project Solar / Phase 1 / Components] AppHeader carbonization#3819dchyun wants to merge 10 commits intoproject-solar/phase-1-main-feature-branchfrom
AppHeader carbonization#3819Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
fcbf194 to
c098983
Compare
c098983 to
4060437
Compare
jorytindall
left a comment
There was a problem hiding this comment.
I did a quick run through of this and caught a couple of things right off the bat, but I think we're going to need to make some decisions about the spacing and visual language of some of the interactive elements. I've left some comments for us to discuss in the planning doc.
| --token-app-header-interactive-surface-color-active: rgba(141, 141, 141, 0.5); | ||
| --token-app-header-interactive-surface-color-default: rgba(0, 0, 0, 0); | ||
| --token-app-header-interactive-surface-color-disabled: #c6c6c6; | ||
| --token-app-header-interactive-surface-color-hover: rgba(141, 141, 141, 0.12); |
There was a problem hiding this comment.
For some reason I'm not seeing the interactive states for the home-link (not sure where else to comment this)
There was a problem hiding this comment.
The Carbon component doesn't have an interactive state for the home link, so I removed it from the HDS home link as well.
b05b7c3 to
b610392
Compare
35b276b to
5f49c8b
Compare
b610392 to
7b51cf0
Compare
87b8462 to
9ba052b
Compare
jorytindall
left a comment
There was a problem hiding this comment.
Sorry @dchyun I thought I had submitted this review, but there's one cleanup item regarding text color, then I think this will be good to go
KristinLBradley
left a comment
There was a problem hiding this comment.
I left a few comments/questions and minor suggestions.
Important
This PR contains a squash of #3814. Do not merge until that PR is merged
📌 Summary
This PR carbonizes the
AppHeadercomponent and adds component tokens for it to the tokens library.Additionally it removes the
hds-interactive-dark-thememixin, which was only used in theAppHeader.AppHeader carbonization page
Note: This PR contains a squash of the Dropdown carbonization in #3814 since the AppHeader uses the Dropdown. Do not merge until that PR is merged, or the squash is removed.
🛠️ Detailed description
Summary of changes:
hds-interactive-dark-thememixinhds-interactive-dark-themeremovalCurrently the
AppHeaderutilizes thehds-interactive-dark-thememixin for its interactive elements. It is the only component that utilizes this mixing currently.I have decided to remove this mixin, and implement all styles within the
AppHeaderfor the following purposes:AppHeadercurrentlyhds-interactive-light-thememixin and switch between using that mixin and thedark-interavctivebased onhds-apply-only-if-carbon🔗 External links
Jira ticket: HDS-6073
Figma file: [if it applies]
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.