Good question @CCiufo-WMF and thanks for laying out the options @egardner - I would recommend we update the markup of the Vue component to use <details> (in a future patch) and use that for the CSS-only version as well. I can't get behind the idea of not using the appropriate, semantic HTML element just to support IE11

I agree we should leave it to a follow up task. We should also consider whether or not CSS-only components should be considered part of an MVP component, or if they should be "planned component updates" that are created at time of MVP component implementation. Is there a task for this already? If not I can create one!

I've included the component's documentation in the task description explaining how we could represent the accordion in the configurable demo and other demos.

Change 921672 merged by jenkins-bot:

[design/codex@main] Accordion: initial implementation of accordion component

https://gerrit.wikimedia.org/r/921672

The initial implementation is now merged (as a WIP component), and can be viewed here. It's ready for an initial design review.

Based on a quick comparison between the spec sheet and the implementation, here's what I think is still missing:

• Add an optional description below the title (this is shown in the spec sheet, but not implemented)
• Implement correct behavior for long titles
  • Titles should wrap, but they currently don't
  • Titles should not go under the end icon, but they currently do (we need to add padding on the title to make room for the absolutely positioned end icon)
• Revisit padding/margin on the content
  • Right now there's 12px padding around the content, but the Figma spec sheet appears to call for 8px on the top (but still 12px on the bottom and on the left+right)

In T326665#8886680, @CCiufo-WMF wrote:

I agree we should leave it to a follow up task. We should also consider whether or not CSS-only components should be considered part of an MVP component, or if they should be "planned component updates" that are created at time of MVP component implementation. Is there a task for this already? If not I can create one!

I have created T338184 for this.

Change 927313 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] Accordion: Add ARIA-labels for icon-only buttons

https://gerrit.wikimedia.org/r/927313

Change 927313 merged by jenkins-bot:

[design/codex@main] Accordion: Add ARIA-labels for icon-only buttons

https://gerrit.wikimedia.org/r/927313

Change 927786 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.11.0 to v0.12.0

https://gerrit.wikimedia.org/r/927786

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/8ca78d2715/w

Change 927786 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.11.0 to v0.12.0

https://gerrit.wikimedia.org/r/927786

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/8ca78d2715/w/

Hey @Sswiergosz, the Design Systems team is wondering if you have the capacity to make a few last updates that came out of the design review for this component (see the updated task description). Once those are complete we can publish the component!

Hi @CCiufo-WMF, sorry for the recent inactivity but I've been on sick leave for the last two weeks. I can work on it in the first half of next week. Does it suit you?

In T326665#8937427, @Sswiergosz wrote:

Hi @CCiufo-WMF, sorry for the recent inactivity but I've been on sick leave for the last two weeks. I can work on it in the first half of next week. Does it suit you?

I'm sorry to hear that, I hope you are feeling better. There's no urgency around this on our end -- just wanted to check in to see if you were still interested in contributing post-hackathon. Next week is perfectly fine!

Thanks, I'm much better now! I'll definitely take care of it next week. Besides the design review changes, I see some other things also changed (e.g. padding around action buttons) and I'll revisit it as well.

Also, I'm not sure why tabindex property was added to the component and set to -1. Because of that, it's impossible to interact with the accordion using a keyboard. @egardner I see that you added tabindex. Could you explain why? Thanks!

In T326665#8905947, @bmartinezcalvo wrote:

Design sign-off done. Adding some things we should fix:

1. The label in the accordion should use the base text that in Codex is 16/22. Updated the spec to reflect this.
2. Could we extend the accordion content to a bigger paragraph? We could use this text from the spec "The accordion content can be as longer as needed, and the type of content can vary according to the need of each use case. Headings should be used to label each section of content."
3. Padding between the arrow and the accordion label should be 8px @spacing-50
4. Accordion content - padding on top should be 8px @spacing-50 (now it's 12px so it makes the accordion content too separated from the label)

Apart form this, I added some notes in the task description about the configurable and standalone demos, could we update them?

Configurable demo should include:

• Description: hide/display it with a toggle switch
• End action prop (icon-only button) with customizable icon if possible
• Disabled: enable/disable the accordion with a toggle switch
• Slot to customize the text in the body content and make it longer if necessary
• LTR/RTL visualization

Other demos in the page should include:

• Customizable label's text style (customize with one of our text tokens, e.g. Heading L 24/30 used in the Minerva's Wikipedia article)
• Represent other body content within the accordion (e.g. an image, a combination of form items, etc.)

---

Moving the task to Ready for development so we can fix the list commented above.

Hi @bmartinezcalvo - could you please explain what do you mean by Customizable label's text style (customize with one of our text tokens, e.g. Heading L 24/30 used in the Minerva's Wikipedia article) ?

I'm not familiar with this text token for heading :)

In T326665#8956681, @Sswiergosz wrote:

In T326665#8905947, @bmartinezcalvo wrote:

Design sign-off done. Adding some things we should fix:

1. The label in the accordion should use the base text that in Codex is 16/22. Updated the spec to reflect this.
2. Could we extend the accordion content to a bigger paragraph? We could use this text from the spec "The accordion content can be as longer as needed, and the type of content can vary according to the need of each use case. Headings should be used to label each section of content."
3. Padding between the arrow and the accordion label should be 8px @spacing-50
4. Accordion content - padding on top should be 8px @spacing-50 (now it's 12px so it makes the accordion content too separated from the label)

Apart form this, I added some notes in the task description about the configurable and standalone demos, could we update them?

Configurable demo should include:

• Description: hide/display it with a toggle switch
• End action prop (icon-only button) with customizable icon if possible
• Disabled: enable/disable the accordion with a toggle switch
• Slot to customize the text in the body content and make it longer if necessary
• LTR/RTL visualization

Other demos in the page should include:

• Customizable label's text style (customize with one of our text tokens, e.g. Heading L 24/30 used in the Minerva's Wikipedia article)
• Represent other body content within the accordion (e.g. an image, a combination of form items, etc.)

---

Moving the task to Ready for development so we can fix the list commented above.

Hi @bmartinezcalvo - could you please explain what do you mean by Customizable label's text style (customize with one of our text tokens, e.g. Heading L 24/30 used in the Minerva's Wikipedia article) ?

I'm not familiar with this text token for heading :)

@Sswiergosz I meant the accordion label text will use the base text style documented in the spec (Bold) but it could be customized with another text style (using any of our Font tokens) to cover other use cases. Anyway, the last two bullets in my list will be finally done in this other task T338821 so you don't need to complete them now.

Change 932412 had a related patch set uploaded (by Sswiergosz; author: Sswiergosz):

[design/codex@main] Accordion: changes after design review

https://gerrit.wikimedia.org/r/932412

Thanks @bmartinezcalvo. I submitted the patch without font tokens for now. I updated the docs to show different body content within the accordion though.

I fixed all of the comments after the Design Review. I also added functionality to wrap too-long titles/descriptions to the next lines.

In T326665#8958558, @Sswiergosz wrote:

Thanks @bmartinezcalvo. I submitted the patch without font tokens for now. I updated the docs to show different body content within the accordion though.

I fixed all of the comments after the Design Review. I also added functionality to wrap too-long titles/descriptions to the next lines.

@Sswiergosz thank you! I've reviewed the new patch and all comments have been fixed except the one related with the customizable demo, including it here:

• Updated the configurable demo including:
  • Description: hide/display it with a toggle switch
  • End action prop (icon-only button) with customizable icon if possible
  • Disabled: enable/disable the accordion with a toggle switch
  • Slot with a text input to type and customize the text in the accordion's body content to make it longer/shorter
  • LTR/RTL visualization

Is it possible to include these items in the configurable demo (the first demo in the page)? We always include a configurable demo in each component's page (e.g. Button) so he user can play with the different properties of the component. So, in this case, the Default demo could be customizable and renamed it to "Configurable demo"

Apart from this, one small thing I would update is the demo with visible icon:

1. I would rename it to "With always visible icon action" since the action is not an icon but a button instead.
2. Although the button's icon can be customized to any action, I would include in this example all edit buttons since, to represent more reality, we will usually have the same actions in the same accordion group.

@bmartinezcalvo sure thing, I can include that configurable demo! I wasn't sure at the beginning what do you mean by that but the examples helped a lot :) I have only one question. You wrote "Description: hide/display it with a toggle switch" but currently the description is passed using a slot. Why do we need such a switch for toggling the visibility of a description? I'm unsure if the switch is needed; no prop in the code handles that.

@Sswiergosz I agree; since the description is a slot, we don't really have a mechanism in the configurable demo system for turning it on or off with a switch. Instead, you can just include it as a slot control (see the configurable demo for the Field component for an example with multiple slots) and people can delete the text there to see the component without a description.

In T326665#8965826, @AnneT wrote:

@Sswiergosz I agree; since the description is a slot, we don't really have a mechanism in the configurable demo system for turning it on or off with a switch. Instead, you can just include it as a slot control (see the configurable demo for the Field component for an example with multiple slots) and people can delete the text there to see the component without a description.

@Sswiergosz yes, you can solve it with the lot control instead as Anne commented. Updated the task description with it. Thank you!

I included the configurable demo. All requirements should be addressed now. Also, I added aria-disabled property to the accordion, somehow I missed that during the development.

In T326665#8968501, @Sswiergosz wrote:

I included the configurable demo. All requirements should be addressed now. Also, I added aria-disabled property to the accordion, somehow I missed that during the development.

Thank you @Sswiergosz! The items in the design review list have been checked. But I've found the following things to fix while reviewing the new configurable demo:

1. Disabled state should not receive hover
2. Since the base case for the accordion will be without a description, I would leave the accordion in the demo without any description (then, the user will be able to type the description in the slot if needed)
   
3. End action should be aligned with the accordion's title and arrow (now the action is centered in the height)
   
4. Accordion's content: I know it can be customized with any content or text size, but it seems weird to view this body as smaller than the title and description. Could we use the same text size for all of them in the demos?

I'm not sure about 3rd point - the end icon is centered in the height but its container has the same height as the title and arrow container and every of that element is centered vertically. Are you sure it's not aligned? Maybe I'm missing something here @bmartinezcalvo

In T326665#8968902, @Sswiergosz wrote:

I'm not sure about 3rd point - the end icon is centered in the height but its container has the same height as the title and arrow container and every of that element is centered vertically. Are you sure it's not aligned? Maybe I'm missing something here @bmartinezcalvo

@Sswiergosz the problem with the icon alignment is when the description is displayed.

Current demo	How it should be

Change 932412 merged by jenkins-bot:

[design/codex@main] Accordion: changes after design review

https://gerrit.wikimedia.org/r/932412

Change 933999 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] Accordion: Simplify styles and add comments

https://gerrit.wikimedia.org/r/933999

@bmartinezcalvo thanks, it should be fixed now

In T326665#8975203, @Sswiergosz wrote:

@bmartinezcalvo thanks, it should be fixed now

@Sswiergosz thank for all the corrections, now all looks good and the Design Review items have been checked.

One last think is about the keyboard navigation, the focus is placed in the accordion item when clicking on it, but it's not placed there when using the tab as in the rest of components (e.g. Button). Could you review this?

@bmartinezcalvo that's actually a good question. It wasn't changed by me. From what I see @egardner added tabIndex = -1 property to the accordion item. I asked about it earlier in this thread but didn't get any response so I'm not sure why it was done/if I should change it.

Change 934412 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] Accordion: ARIA fixes and improvements

https://gerrit.wikimedia.org/r/934412

@Sswiergosz and @bmartinezcalvo, it looks like I made a mistake adding tabindex=-1 here – I did so based on a misunderstanding of the markup at an earlier stage of this component's development.

I've just posted a new patch that should address this problem: https://gerrit.wikimedia.org/r/934412.

In T326665#8978299, @egardner wrote:

@Sswiergosz and @bmartinezcalvo, it looks like I made a mistake adding tabindex=-1 here – I did so based on a misunderstanding of the markup at an earlier stage of this component's development.

I've just posted a new patch that should address this problem: https://gerrit.wikimedia.org/r/934412.

@egardner thank you! The focus works properly now. So the accordion's corrections are all done @Sswiergosz. Thank you both!

Change 933999 merged by jenkins-bot:

[design/codex@main] Accordion: Simplify styles and add comments

https://gerrit.wikimedia.org/r/933999

Change 934412 merged by jenkins-bot:

[design/codex@main] Accordion: ARIA fixes and improvements

https://gerrit.wikimedia.org/r/934412

Change 935794 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.13.0 to v0.14.0

https://gerrit.wikimedia.org/r/935794

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/64dc3126c0/w

Change 935794 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.13.0 to v0.14.0

https://gerrit.wikimedia.org/r/935794

@CCiufo-WMF since the accordion has been successfully implemented in Codex and all corrections have been fixed, I guess we can solve this task.

In T326665#8992724, @bmartinezcalvo wrote:

@CCiufo-WMF since the accordion has been successfully implemented in Codex and all corrections have been fixed, I guess we can solve this task.

Not quite. The component is complete but it is still marked as a "WIP" component, so it is not available in the latest Codex release. I know @Catrope mentioned looking into this and can maybe weight in on next steps.

We have this documentation on transitioning the component from WIP --> Public.

Change 936087 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] Accordion: Move from WIP components to public components

https://gerrit.wikimedia.org/r/936087

Change 936087 merged by jenkins-bot:

[design/codex@main] Accordion: Move from WIP components to public components

https://gerrit.wikimedia.org/r/936087

Related: T342031: Accordion: Remove "disabled" property for now and T342063: Accordion: Allow the accordion to be initially open, and/or programmatic opening/closing

Change 939386 had a related patch set uploaded (by Jforrester; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.14.0 to v0.15.0

https://gerrit.wikimedia.org/r/939386

Change 939386 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.14.0 to v0.15.0

https://gerrit.wikimedia.org/r/939386

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/64dc3126c0/w/