Skip to content
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

New DS core component: accordion #632

Merged
merged 11 commits into from
Apr 13, 2023
Merged

New DS core component: accordion #632

merged 11 commits into from
Apr 13, 2023

Conversation

vmilan
Copy link
Contributor

@vmilan vmilan commented Apr 10, 2023

Description

Shortcut: (302856)

Implement this core component needed by Workflows product.

@vmilan vmilan changed the title New core component: accordion New DS core component: accordion Apr 10, 2023
@vmilan vmilan marked this pull request as ready for review April 13, 2023 07:31
@vmilan vmilan requested a review from a team April 13, 2023 07:32
@coveralls
Copy link
Collaborator

coveralls commented Apr 13, 2023

Pull Request Test Coverage Report for Build 4689752857

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.345%

Totals Coverage Status
Change from base Build 4689734058: 0.0%
Covered Lines: 1949
Relevant Lines: 2513

💛 - Coveralls


...(variant === 'outlined' && {
backgroundColor: theme.palette.background.paper,
boxShadow: `inset 0 0 0 1px ${theme.palette.divider}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity.

We've got theme.shadows. Are they of no use here? If not, when they should be used (they are used in extensively in cloud-native)

Copy link
Contributor Author

@vmilan vmilan Apr 13, 2023

Choose a reason for hiding this comment

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

This boxShadow use is custom, to simulate a border indeed... Designers wanted to have this line inside the element itself, and using an inset border didn't work properly.

But yes, we use theme.shadows for common use cases.

const Template = ({ ...args }) => {
return <Avatar {...args} />;
};

const ShapeTemplate = ({ ...args }) => {
const classes = useStyles();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to use the opportunity for cleanup!

Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

👍🏻

@vmilan vmilan merged commit 8b67758 into master Apr 13, 2023
@vmilan vmilan deleted the feature/accordion branch April 13, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants