refactor(accordion): new helm api

thatsamsonkid commented 7 months ago

PR Checklist

Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

Which package are you modifying?

[x] accordion
[ ] alert
[ ] alert-dialog
[ ] aspect-ratio
[ ] avatar
[ ] badge
[ ] button
[ ] calendar
[ ] card
[ ] checkbox
[ ] collapsible
[ ] combobox
[ ] command
[ ] context-menu
[ ] data-table
[ ] date-picker
[ ] dialog
[ ] dropdown-menu
[ ] hover-card
[ ] input
[ ] label
[ ] menubar
[ ] navigation-menu
[ ] popover
[ ] progress
[ ] radio-group
[ ] scroll-area
[ ] select
[ ] separator
[ ] sheet
[ ] skeleton
[ ] slider
[ ] switch
[ ] table
[ ] tabs
[ ] textarea
[ ] toast
[ ] toggle
[ ] tooltip
[ ] typography

What is the current behavior?

Current hlm accordion api is this

<div hlmAccordion>
    <div hlmAccordionItem>
          <button hlmAccordionTrigger>
            Is it accessible?
            <hlm-icon hlmAccIcon />
          </button>
          <brn-accordion-content hlm>
             Yes. It adheres to the WAI-ARIA design pattern.
          </brn-accordion-content>
      </div>
     ...
</div>

What is the new behavior?

This PR simply expands on the above to provide a slightly simpler hlm api with the option to do something like the below. Looks a little cleaner and a little more consistent from a selector/tag perspective. But you can still continue to do either implementation depending on preference or use case

<hlm-accordion>
      <hlm-accordion-item>
        <button hlmAccordionTrigger>
          Is it accessible?
          <hlm-icon hlmAccIcon />
        </button>
        <hlm-accordion-content>
          Yes. It adheres to the WAI-ARIA design pattern.
        </hlm-accordion-content>
      </hlm-accordion-item>
     ...
</hlm-accordion>

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

thatsamsonkid commented 7 months ago

Oh no, you know I thought I remembered having kept that selector for hlm-accordion-item.directive. Shoot do we want to add it back? @goetzrobin @elite-benni I really meant to keep it none breaking

goetzrobin commented 7 months ago

Haha all good! Let's add it back! Nothing's released yet 😃

elite-benni commented 7 months ago

Its already merged and we are in alpha ;) We do many breaking changes, while reworking the api. I don't think it is a problem, just wanted to point out that it is a breaking change.

goetzrobin / spartan