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

[86bye3tve][accordion] added use prop #1474

Merged
merged 7 commits into from
Jul 12, 2024
Merged

Conversation

msereniti
Copy link
Contributor

Motivation and Context

For a long time our design has both compact and non compact accordions. Alongside react component were not supporting non compact accordion without hacks. This PR adds simple compact prop that is true by default for backward capability.

How has this been tested?

Manually and with VR.

Screenshots (if appropriate):

Screenshot 2024-06-27 at 17 43 14

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Nice improve.

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly or it's not required.
  • Unit tests are not broken.
  • I have added changelog note to corresponding CHANGELOG.md file with planned publish date.
  • I have added new unit tests on added of fixed functionality.

semcore/accordion/src/index.d.ts Outdated Show resolved Hide resolved
semcore/accordion/src/index.d.ts Outdated Show resolved Hide resolved
@j-mnizhek
Copy link
Contributor

  1. This accordion should be secondary not primary, since it usually uses less important place in the interface hierarchy.
image
  1. Can we add margin-bottom: var(--intergalactic-spacing-05x) (or gap) by default for the ItemToggle for the primary accordion (as it was in the design)? The last child of course shouldn't have it.
image

@j-mnizhek j-mnizhek self-requested a review July 4, 2024 16:48
Copy link
Contributor

@j-mnizhek j-mnizhek left a comment

Choose a reason for hiding this comment

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

Suggested some changes in the comment to the PR.

@ilyabrower
Copy link
Contributor

ilyabrower commented Jul 5, 2024

@msereniti

  1. Can we add margin-bottom: var(--intergalactic-spacing-05x) (or gap) by default for the ItemToggle for the primary accordion (as it was in the design)? The last child of course shouldn't have it.

I've fixed it in #1468

@msereniti msereniti changed the title [86bye3tve][accordion] added compact mode prop [86bye3tve][accordion] added use prop Jul 10, 2024
@msereniti msereniti merged commit ac1aabe into master Jul 12, 2024
9 checks passed
@msereniti msereniti deleted the accordion-non-compact branch July 12, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants