-
Notifications
You must be signed in to change notification settings - Fork 5
Add UI Blocks to Context Panel #1470
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
ed2e0a6 to
e08a5ab
Compare
e08a5ab to
54e3528
Compare
54e3528 to
e6ea5d1
Compare
e6ea5d1 to
87a5600
Compare
ecc2b18 to
ebfc92f
Compare
87a5600 to
8f16f7a
Compare
ebfc92f to
f40863c
Compare
8f16f7a to
a526425
Compare
d385e99 to
66a011a
Compare
1d45169 to
1cb8f58
Compare
4814e7c to
4821940
Compare
7f0ee6f to
f3260f1
Compare
436b867 to
d89ccdb
Compare
6dcfcb1 to
c063c0a
Compare
| }, | ||
| }); | ||
|
|
||
| export type IconName = keyof typeof icons; |
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.
love it!
c063c0a to
d93cd57
Compare
da65e9c to
abf1484
Compare
abf1484 to
49b752e
Compare
maxy-shpfy
left a comment
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.
LGTM!
| <CollapsibleTrigger asChild> | ||
| <Button variant="ghost" size="sm"> | ||
| <Icon name="ChevronsUpDown" /> | ||
| <span className="sr-only">Toggle</span> |
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.
For future: interesting, that we may need to take it further one day, and apply a11y
| <BlockStack className={className}> | ||
| <Collapsible className="w-full"> | ||
| <InlineStack blockAlign="center" gap="1"> | ||
| {title && <Heading level={3}>{title}</Heading>} | ||
| {collapsible && ( | ||
| <CollapsibleTrigger asChild> | ||
| <Button variant="ghost" size="sm"> | ||
| <Icon name="ChevronsUpDown" /> | ||
| <span className="sr-only">Toggle</span> | ||
| </Button> | ||
| </CollapsibleTrigger> | ||
| )} | ||
| </InlineStack> | ||
|
|
||
| {collapsible ? ( | ||
| <CollapsibleContent className="w-full mt-1"> | ||
| {content} | ||
| </CollapsibleContent> |
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: this block looks really similar to the ContentBlock - maybe we can re-use code pieces?
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.
I thought about it but didn't want to over-complicate things. We can do as a follow-up if we feel it's necessary
49b752e to
a8add58
Compare
| const key = | ||
| isValidElement(action) && action.key != null | ||
| ? `action-node-${String(action.key)}` | ||
| : `action-node-${index}`; |
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.
I'm ok with this for now (since in nature action list is pretty static, so we should not see issues related to key messed up), but I would revisit and see if we can use smth like "ID" on the Action structure
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.
In the long-term this may likely be removed - i.e. in #1544

Description
Adds standard UI blocks for use inside the ContextPanel. Hopefully this will allow us a bit more visual consistency going forward. Each block has a header/title and some content. The header is the same for all blocks, but the content will be different depending on the block.
Implementation of the Blocks into existing Context Panel use cases is done upstack.
Blocks Added:
ActionBlock: Renders a horizontal array of identical buttons based on an input label, icon and callback. Custom configuration options allow buttons to be hidden or disabled upon certain conditions, or to have two-step confirmation.ContentBlock: Renders any given ReactNode content. An optional prop allows the block to be collapsed.ListBlock: Renders a list of key:value pair items. Items can be with or without markers. Items without a value will not render in the listTextBlock: Renders text. Text can be copyable or collapsible via optional propsAdditionally, the
Attributecomponent was added, which simply renders a given label and associated value. Value can be a string or a link.This PR also adds unit tests to the new Blocks.
Related Issue and Pull requests
https://github.com/Shopify/oasis-frontend/issues/401
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
This PR adds new UI blocks but does not implement them in the interface. If you want to test them and see what they look like visual:
(wouldn't it be neat to have a UI playground?)
Additional Comments