-
Notifications
You must be signed in to change notification settings - Fork 69
[LG-5293] feat: ContextDrawer #2918
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
🦋 Changeset detectedLatest commit: c1cb077 The changes in this PR will be included in the next version bump. This PR includes changesets to release 85 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Change: +134 B (+0.01%) Total Size: 1.88 MB
ℹ️ View Unchanged
|
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.
Small nit and question, but no blockers, LGTM!
import { getFill, getWrapperStyles } from './ButtonCorner.styles'; | ||
import { Side } from './ButtonCorner.types'; | ||
|
||
export const ButtonCorner = ({ side }: { side: Side }) => { |
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.
Nit: took me a sec to get what a button corner was, had to look at how it was used. Perhaps a small comment as simple as "visually rounded sides of the ContextDrawerButton
" would make this super clear at a glance.
&:focus-visible { | ||
background-color: ${backgroundColor}; | ||
color: ${textColor}; | ||
box-shadow: none; | ||
} | ||
|
||
&:hover, | ||
&:active { | ||
background-color: ${backgroundColor}; | ||
color: ${textColor}; | ||
box-shadow: none; | ||
} |
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.
Q: We don't want anything to change visually on focus or hover? Any reason why?
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.
this is according to the design specs here: https://www.figma.com/design/4h2VwjCuJUbeZ7hzD2J1rq/LeafyGreen-Design-System?m=auto&node-id=41064-134808
I explicitly set these here to reset the styling applied in LG button
…y for styling ContextDrawerButton
a8b8a28
to
a9c690b
Compare
* chore(context-drawer): update dependencies * fix(context-drawer): ContextDrawerButton glyph rotates * feat(context-drawer): implement ContextDrawer component with stories and specs * docs(context-drawer): README * fix(ContextDrawerButton): styling * refactor(ContextDrawer): hardcode additional spacing for bottomInterceptRef * style(ContextDrawer): improve visibility transition and collapse reference border * docs(context-drawer): README format * refactor(context-drawer): tab navigation with specs * update spec * Reduce redundant css selector specificity * feat(lib): add formatCssSize util (#2930) * feat(lib): add formatCssSize util * refactor(ContextDrawer): formatCssSize on expandedHeight * chore(lib): changeset * refactor(lib): formatCssSize trims string values before format * refactor(lib): formatCssSize types
✍️ Proposed changes
This PR covers part 1 of the
ContextDrawer
:@leafygreen-ui/context-drawer
packageContextDrawerButton
componentButtonCorner
component🎟 Jira ticket: LG-5293
✅ Checklist
For new components
🧪 How to test changes
Components/ContextDrawer/ContextDrawerButton
live example storyComponents/ContextDrawer/ContextDrawerButton
generated stories