Enterprise Nav: AppHeader component (HDS-3373)
#2161
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| aria-label={{this.ariaLabel}} | ||
| > | ||
| <FlightIcon @name={{@icon}} @color={{@color}} @stretched={{true}} /> | ||
| </Hds::Interactive> |
There was a problem hiding this comment.
This is pretty much the same as the SideNav::HomeLink component
| // constructor() { | ||
| // super(...arguments); | ||
| // // ADD YOUR ASSERTIONS HERE | ||
| // } |
There was a problem hiding this comment.
Will clean all this up later.
| </:globalAfter> | ||
|
|
||
| <:utility> | ||
| <Hds::Button class="hds-side-nav__icon-button" @icon="search" @isIconOnly={{true}} @text="Search" /> |
There was a problem hiding this comment.
Icon only buttons icon size vs. Dropdown icon size needs to be more closely examined and perhaps reworked. (Examined & explored this and discussed with @jorytindall.)
I also am now realizing that the "one-off" SideNav::Header::IconButton was likely a mistake that we should also rethink. http://localhost:4201/components/side-nav#sidenav-header-iconbutton
We should look to come up with a better strategy for implementing the dark "theme" for the Button and Dropdown as well. (@jorytindall as a reminder)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
The Hds::SideNav::Header::IconButton forces a non-standard icon size (in addition to adding dark theme styling). It was created to follow custom styling Terraform had been using for their search button. In my mind, needing a special component to change the button icon size highlights a possible design issue which should be solved in the base Button and Dropdown components. @jorytindall and I can look at this though and decide what to do.
This is the AppHeader using standard icon sizes:
|
@jorytindall and I discovered a possible design issue with the sizing of icons in the icon only On the right side in the "utility" items area, observe that the search icon in the button is noticeably smaller than the icons in the dropdowns next to it. For the SideNav, we had ended up creating a 2 issues to consider:
|
There was a problem hiding this comment.
Left one initial note (I know this is a draft) on the focus/active state of the icon-only buttons being overridden. This is looking really solid so far!
One thing to consider is the width of the various "pickers" (context switcher/geo picker):
- Since they aren't within the
SideNavanymore, what should the width of these elements be? - Should they adapt to the content within (and change when something longer/shorter is selected)?
- Should we set a min/max width? That might result in a long label getting cut off, or requiring truncation 👎
I'll think about this a bit, but just highlighting as another topic of discussion. Great job getting this throw together so quickly!
| </:globalAfter> | ||
|
|
||
| <:utility> | ||
| <Hds::Button class="hds-side-nav__icon-button" @icon="search" @isIconOnly={{true}} @text="Search" /> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
KristinLBradley
force-pushed
the
hds-3373-app-header-component
branch
from
bcd15f6
to
06c6325
Compare
|
|
||
| // Used to apply dark theme to interactive elements such as Button & Dropdown | ||
| @mixin hds-interactive-dark-theme($add-visible-border: true, $is-within-side-nav: false) { | ||
| @if $is-within-side-nav { |
There was a problem hiding this comment.
It's not that bad, at least it's hidden all in one place
| // general dark theme tokens | ||
| color: var(--token-color-palette-neutral-0); | ||
| background-color: var(--token-color-palette-neutral-700); | ||
| border-radius: var(--token-form-control-border-radius); |
There was a problem hiding this comment.
technically it's not the form-control-border-radius but something like button-border-radius; unfortunately we don't have such token, only a variable $hds-button-border-radius in the mixins/_button.scss, maybe you can import the mixin and use that variable instead? (I've never tried, but probably should work)
There was a problem hiding this comment.
Ok, will see. (I picked that one because it seemed closest.)
…with AppHeader, Add styling to remove excess spacing from empty header and footer
…wer design mocks, change buttons in Showcase to use Hds::Button icon only variant
…:empty selector to remove excess padding in empty elements
…nd shift items around to match mocks
…xin, apply styles to child components via inheritance, add examples of interactive child components to Showcase
…der and SideNav to use for interactive components
KristinLBradley
force-pushed
the
hds-3373-app-header-component
branch
from
1b2ce4a
to
e3c5b21
Compare
STATUS: Created preliminary component(s) & tests. In process of refining components, APIs, & fleshing out Showcase examples. Visual styles, including for Button & Dropdown content, are nearing finalization. Responsive behavior & styling NOT yet completed.
NOT yet ready for in depth review. Sharing for visibility and initial high level feedback on open questions.
📌 Summary
If merged, this PR adds the
AppHeadercomponent to HDS.Showcase:
🛠️ Detailed description
Changes include:
AppHeadercomponent, including:SideNavto coordinate withAppHeader🔗 External links
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.