Skip to content

Conversation

@MezzyCode
Copy link

Summary:

Fixed expander glitch on Firefox and Edge (not as rough). Simplifies the logic in expandCard. Commented out rotate() helper function in favor of pure css animation. Also added aria-expanded for accessibility.

What changed:

  • Comment out rotate() helper and nuke the calls.
  • Rewrote expandCard, simplified and make it use aria-expanded on trigger element.
  • Tune the css a bit so that it works with the modified expandCard.

Why:

  • Expander goes glitchy on Firefox and Edge, i believe it's because of mixing display toggles with CSS height transition. Changed it to use opacity and max-height instead for the fix.
waaa.mp4
wooo.mp4
  • Used aria-expanded because it improves accessibility a bit.

Notes & Possible follow-ups:

Direct inline styles is pretty dirty but its hard to do clean on current element structure. I'm suggesting to change the expander structure from:

  <div class="expander-top">
    <p>
    <h3>_</h3>
    <img class="chevron" style="transform: rotateX(0deg);" src="img/UI/Chevron Down.svg" alt="UI element - Chevron">
    </p>
  </div>
  <div class="expander-bottom">
    <p> _ </p>
  </div>

to:

  <details class="expander">
    <summary class="expander-header">
      <h3>Header...</h3>
    </summary>
    <img class="chevron" src="img/UI/Chevron Down.svg" alt="UI element - Chevron">
    <div class="expander-content">Content...</div>
  </details>

With that it's possible to put css variable on .expander and do something like el.style.setProperty('--expander-height', val). New structure also gave even more accessibility too. Just gave the green light and I'll cook them up.

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.

1 participant