-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat(components): listButton and accordion children creation #15967
Conversation
background-color: ${disabled | ||
? COLORS.grey35 | ||
: listButtonProps.backgroundColor}; | ||
max-width: 26.875rem; |
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.
Could you tell me where this info is?
|
||
&:hover { | ||
background-color: ${listButtonProps.hoverBackgroundColor}; | ||
} |
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.
no need focus-visible
?
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.
we can add this later, the component isn't quite design finalized yet! we will have to go back and do DQA on all these components
|
||
const LIST_BUTTON_STYLE = css` | ||
cursor: pointer; | ||
background-color: ${disabled |
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 not sure that the design team is thinking of using aria-disabled
for desktop app/web app, but if there is possibility, supporting aria-disabled
now would be easy for us.
@koji yes that is it! this is the main component, i think it has a bit more info: https://www.figma.com/design/OIdG64Q5cgvEw82ish5Eee/Design-System%3A-Flex?node-id=1850-148196&t=gebqvUP69jVOIWEe-0 |
components/src/atoms/ListButton/ListButtonChildren/ListButtonAccordion.tsx
Outdated
Show resolved
Hide resolved
components/src/atoms/ListButton/ListButtonChildren/ListButtonAccordion.tsx
Outdated
Show resolved
Hide resolved
components/src/atoms/ListButton/ListButtonChildren/ListButtonAccordion.tsx
Outdated
Show resolved
Hide resolved
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.
Adding text like 'click the button to see Accordion, Radio, Nested' would be user-friendly for the designers.
we can notice that easily since we read code, but they don't check code.
probably adding text might DQA smooth.
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.
left some comments.
once they are resolve, this pr should be good to go.
closes AUTH-657
Overview
Create
ListButton
,ListButtonAccordion
,ListButtonRadioButton
andListButtonAccordionContainer
to be used in PD redesignTest Plan and Hands on Testing
it is not used yet so check the storybook. should be able to expand the accordion and pressing on the buttons should also expand an accordion if it is nested
Changelog
create all the above components mentioned above and add a story. also add tests for each
Review requests
Risk assessment
low