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

AccordionItem styles for container are not applied #23

Open
perruche opened this issue Feb 16, 2022 · 4 comments
Open

AccordionItem styles for container are not applied #23

perruche opened this issue Feb 16, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@perruche
Copy link
Contributor

perruche commented Feb 16, 2022

The container of the AccordeonItem cannot be styles via data-option-styles

Step to reproduce

Given this code, which is 95% a copy from the exemple.
When open/closed the custom btn class are applied, but not the container ones

{% embed '@ui/molecules/Accordion/Accordion.twig'
    with {
      items: items,
      item_attr: {
        data_option_styles: {
          btn: {
            open: 'bg-grey-light rounded-t'
          },
          container: {
            open: 'bg-red',
            active: '',
            closed: 'rounded'
          }
        }
      }
    }
  %}
    {% block title %}
      <div class="px-6">
          {{ item.title }}
      </div>
    {% endblock %}
    {% block content %}
      <div class="p-6">
        {{ item.content }}
      </div>
    {% endblock %}
  {% endembed %}

Possible solution:

It seems to be missing some object spreading on the container transition ?

  async open() {
    // ...
    const { container, ...otherStyles } = this.$options.styles;

    /** @type {AccordionItemRefs} */
    const refs = this.$refs;

    await Promise.all([
      transition(refs.container, {
        from: { height: 0 }, // Missing ...container.closed
        active: container.active,
        to: { height: `${refs.content.offsetHeight}px` }, // Missing ...container.open
      }).then(() => {
        return Promise.resolve();
      }),
      ...Object.entries(otherStyles)
        .filter(([refName]) => refs[refName])
        .map(([refName, { open, active, closed } = { open: '', active: '', closed: '' }]) =>
          // ...
        ),
    ]);
  }
@perruche perruche added the bug Something isn't working label Feb 16, 2022
@titouanmathis
Copy link
Contributor

This was intended in the first draft of the component to avoid overriding the required height transition. The content ref should be used instead to define custom styles for the closed and open states.

We could find a workaround to allow custom styles on the container ref if it is really needed.

@perruche
Copy link
Contributor Author

It is not mandatory to be able to style the container, workarounds with content & btn refs are okay.
But the source code is maybe missleading in AccordionItem.js with this declaration:

styles: {
  type: Object,
  /**
   * @return {AccordionItemStylesOption}
   */
  default: () => ({
    container: {
      open: '',
      active: '',
      closed: '',
    },
  }),
},

Because if you override it, it does nothing (for the ref container)

@perruche
Copy link
Contributor Author

This is now possible to have a fluid transition using grid-template-rows
cf: https://codepen.io/DHawku/pen/vYzLNVK?editors=1100

Shall we implement it as the default behavior ?

@studio-meta
Copy link

oh yeah !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants