Skip to content

Conversation

michaelokolo
Copy link

Description

This pull request introduces a new feature that allows changing the heading tag of an Accordion Item Header.

Related issue: #4999

Changes

  1. Updated metaAccordionHeader in accordion.ws.ts to include h1h6 options, enabling users to select the heading level in the Item Header settings.
  2. Updated AccordionHeader in accordion.tsx to accept and render h1h6 tags dynamically.
  3. Moved the margin-top and margin-bottom styles from metaAccordionHeader to AccordionHeader to ensure consistent spacing across all heading levels.

How to Test

  1. Open any project.
  2. Add an Accordion to the canvas.
  3. Click an Accordion Item Header. In the right-hand settings panel, you should see a new Tag dropdown with options h1–h6.
  4. Select different tags and verify the rendered heading levels in the browser’s developer tools.

h2
h5
Tag

@michaelokolo michaelokolo changed the title Feat/accordion header tag 4999 feat/accordion header tag 4999 Oct 14, 2025
@michaelokolo michaelokolo changed the title feat/accordion header tag 4999 feat: accordion header tag 4999 Oct 14, 2025
@michaelokolo michaelokolo changed the title feat: accordion header tag 4999 feat: accordion header tag (#4999) Oct 14, 2025
type Props = ComponentPropsWithoutRef<typeof Header> & { tag?: Tag };
export const AccordionHeader = forwardRef<HTMLHeadingElement, Props>(
({ tag: legacyTag, ...props }, ref) => {
const tag = getTagFromProps(props) ?? legacyTag ?? defaultTag;
Copy link
Member

Choose a reason for hiding this comment

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

Call variable in pascal case "Heading" and you can use it in jsx without createElement which is considered legacy.

Copy link
Author

@michaelokolo michaelokolo Oct 15, 2025

Choose a reason for hiding this comment

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

Alright, I will resolve this. Thank you.

Copy link
Author

Choose a reason for hiding this comment

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

Hello @TrySound , I’ve made the necessary updates based on your earlier comments.

Comment on lines 65 to 69
style: {
...(props.style || {}),
marginTop: 0,
marginBottom: 0,
},
Copy link
Member

Choose a reason for hiding this comment

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

Builder does not support "style" property. So please remove it. Otherwise it will override user preferences from tokens.

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I will resolve this too. There is a margin-top: 0 and margin-bottom: 0 applied to the H3 Item Header in accordion.ws.ts, which is why I applied the style here. I wanted to ask, do we still need these styles?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think they still needed. At least to not break existing projects.

Copy link
Author

Choose a reason for hiding this comment

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

And just to confirm, do we need this margin-top: 0 and margin-bottom: 0 for all heading levels or just for H3? I am planning to apply it to all heading levels in accordion.ws.ts.

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.

2 participants