Skip to content
This repository has been archived by the owner on Mar 29, 2024. It is now read-only.

220 remove selected state for section headers in nav drawer list #221

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

TristanHoladay
Copy link
Contributor

No description provided.

…ed appStatesStore to only track currentRoute now.
…d route transform function to makeLinkText().
@TristanHoladay TristanHoladay linked an issue May 29, 2023 that may be closed by this pull request
4 tasks
@netlify
Copy link

netlify bot commented May 29, 2023

Deploy Preview for unicorn-ui ready!

Name Link
🔨 Latest commit 10b0108
🔍 Latest deploy log https://app.netlify.com/sites/unicorn-ui/deploys/6488a86c01e13600087cffcf
😎 Deploy Preview https://deploy-preview-221--unicorn-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

…moved style block of ListSubheader.svelte since style is controlled by parent container (i.e. list or drawer); ListSubheader Props now of type BoxProps<HTMLHeadingElement>
@TristanHoladay
Copy link
Contributor Author

TristanHoladay commented May 31, 2023

Screenshot 2023-06-01 at 1 21 28 PM

@TristanHoladay TristanHoladay marked this pull request as ready for review May 31, 2023 16:08
@mike-winberry mike-winberry requested a review from Madeline-UX June 6, 2023 22:08
.drawer.mdc-drawer .mdc-deprecated-list-group__subheader {
color: var(--on-background);
color: rgba(255, 255, 255, 0.7);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be a css var to work on both themes. I believe it already exists in the palette.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also are we documenting these changes in Drawer? These really should just be website changes and not necessary involve the components

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we should be hardcoding the width here as that is something the consumer should be doing. In the case of the drawer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see a corresponding color in the default palettes originally, although text-primary-on-background is close (.87 instead of .7 opacity). I guess i should check the hex colors again to make sure on of those isn't the correct one.

The thing i found was that mdc targets the subheader in the drawer and turns the color really dark ( rgba(0,0,0,.6) ) and that overrides whatever color the subheader is set to. I figured overriding their override as the default style shipped with the drawer component would be the right idea, but if you think that's the wrong direction i'm cool with yanking that and just overriding it for the website.

Yeah you're right about the width. I think i was just setting defaults because the original figma for Drawer shows 320 as well.

Copy link

@Madeline-UX Madeline-UX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hover state is no longer detectable.
Also please take a look at the design for the top nav bar. There should be an elevation applied so it doesn't look like the left nav and top nav are one L piece of papger

@TristanHoladay
Copy link
Contributor Author

@Madeline-UX i'm wondering if something's off between what you're seeing and what I'm seeing. I'm still seeing the hover state for the items in the drawer. The hover state is really dark in dark-mode, though, which is the case on the current release as well. So maybe I just need to fix that hover color.

Also, the figma I think you're referencing (https://www.figma.com/file/aNnt9Ip7IFTs9hnfqrYGl4/Unicorn-UI?type=design&node-id=108-14353&t=iMeXWcDQIgdewqwV-0) shows them both with elevation of 2. Sorry if that's the wrong design.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove selected state for section headers in nav drawer list
3 participants