oncode / handorgel

Accessible W3C conform accordion written in ES6.
https://oncode.github.io/handorgel/
MIT License
261 stars 25 forks source link

Add checks for children elements #9

Closed artifex404 closed 5 years ago

artifex404 commented 5 years ago

I've added additional checks for Handorgel child elements to prevent a JS error when adding an additional element to the structure.

For example I had this structure which throwed an error:

  <h3 class="handorgel__header">
    <button class="handorgel__header__button" autofocus>
      Title (autofocused)
    </button>
  </h3>
  <div class="handorgel__content" data-open>
    <div class="handorgel__content__inner">
      Content openened by default
    </div>
  </div>
  <h3 class="handorgel__header">
    <button class="handorgel__header__button">
      Title 2
    </button>
  </h3>
  <div class="handorgel__content">
    <div class="handorgel__content__inner">
      Content closed by default
    </div>
  </div>
  <div class="handorgel__last-border"></div>
</div>

I've added an additional element since it's not possible to add a border after header, which should be visible after the content when expanded.

artifex404 commented 5 years ago

Sorry I couldn't run the npm tasks to run minifications, since they failed in my environment.

oncode commented 5 years ago

Hi @artifex404

Thank you for the pull request, I'll check it. But for the border you want to achieve there is a solution without adding an extra element:

// set the border on content (for expanded view)
.handorgel__content,
// set the same border to header or header button for when content is not expanded:
.handorgel__header:not(.handorgel__header--open) {
   border-bottom: 5px solid red;
}

Demo: http://recordit.co/S5ZIzmFC39

oncode commented 5 years ago

I merged the check for the header and content even though it would still be a problem if you add two extra elements instead of just one, since the two elements would be initialized as a fold instance. Also the fold push has to be outside of the if condition to be able to add already existing fold instances (I've added comments to make it more clear).

artifex404 commented 5 years ago

yep, that was more like a quick fix. I think the proper solution to detect the fold should check for the classes on the child elements and create the fold only if does match them?