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

Accordion issues fixed #178

Merged
merged 4 commits into from
Jun 16, 2023
Merged

Accordion issues fixed #178

merged 4 commits into from
Jun 16, 2023

Conversation

tishoyanchev
Copy link
Contributor

@tishoyanchev tishoyanchev commented Jun 14, 2023

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

Fixes the following issues with the Accordion:

  • Rename IfxAccordion with no prefix.
  • arrow is facing down by default, and up when list is opened.
  • there should be 8px space between the accordion-items.
  • there is no prop option for auto close an accordion-item when another one is opened.
  • itemOpened and itemClosed event handlers not working on React.
  • when you write a long sentence without space, e.g. 'wwwwwwwwwwwww', it's not being broken into
    multiple lines, instead, it overflows, but the overflow is just hidden. It should break into another line.
  • The componentDidRender lifecycle method is called every time the component renders, even if
    there are no changes to the open property.
  • async close() there is no need for async if await is not used inside.
  • .accordion-content ::slotted(p) {
    margin: 0;
    padding: 0;
    }
    the component should not style this paragraph, since it is not a given that it will be in the
    accordion-item. Users may just add text without

    . The

    is not explicetly part of the
    component.

  • :host {
    display: block;
    }

:host { display : block; } is not needed.

  • @listen('ifxItemOpen', { target: 'body' }) event listener should not be attached to the body.
📦 Published PR as canary version: 18.2.4--canary.178.465a39746b1762aff99655414a8cf5e542dc0105.0

✨ Test out this PR locally via:

npm install @infineon/infineon-design-system-stencil@18.2.4--canary.178.465a39746b1762aff99655414a8cf5e542dc0105.0
# or 
yarn add @infineon/infineon-design-system-stencil@18.2.4--canary.178.465a39746b1762aff99655414a8cf5e542dc0105.0

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-06-16 09:41 UTC

PR Preview URL for React example: https://Infineon.github.io/infineon-design-system-stencil/pr-preview-react-example/pr-178/

Copy link
Contributor

@kaiwerther kaiwerther left a comment

Choose a reason for hiding this comment

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

looks good! two small things

@Listen('ifxItemOpen')
async onItemOpen(event: CustomEvent) {
const items = Array.from(this.el.querySelectorAll('ifx-accordion-item'));
if(this.autoCollapse) {
Copy link
Contributor

Choose a reason for hiding this comment

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

small performance fix: move this line one layer up so we don't query when auto collapse is false

@tishoyanchev tishoyanchev merged commit 297ecc0 into master Jun 16, 2023
4 checks passed
@github-actions
Copy link
Contributor

🚀 PR was released in v18.2.4 🚀

@verena-ifx verena-ifx deleted the accordion-fixes branch October 9, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants