search

Financial-Times / origami

The Origami Component System
58 stars 14 forks source link

Deprecate o-toggle? #159

Open notlee opened 5 years ago

notlee commented 5 years ago

The hidden option of o-expander also allows content to be toggled between hidden and shown states but it doesn't have any callback functionality outside events.

To reduce maintenance and increase the distinction between these: Should o-toggle be deprecated? Or should the hidden option be removed from o-expander? Look into how these components are used to make a decision.

notlee commented 4 years ago

o-toggle recieved Slack praise "o-toggle is an 11 on the :heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat::heart_eyes_cat: scale"

chee commented 2 years ago

my opinion is that o-expander is too complex, and it has features that i couldn't find being used anywhere. i think we can drop data-o-expander-shrink-to and data-o-expander-item-selector. it has the option to shrink to a specific max-height, and a specific number of list items, and an option to disable setting aria-attributes. i'm not sure it should have those. i think it would be great to talk to some designers about toggleable content. i guess we call these something like a disclosure component.

also i think possibly the code for o-toggle is too complex, i feel like we could write something that is smaller and simpler and more robust for this and doesn't have two separate javascript classes for the toggler and the target.

a simpler expander looks like:

a toggle looks like:

they are quite different, but without guidance it might be confusing which one to use. they are both essentially a custom implementation of the native <details/> disclosure element which actually exists already in all our supported browsers (it used to have accessibility problems but i'm not sure it still does). i personally prefer the o-toggle shape if we could maybe have it redesigned slightly (perhaps it should have a ▶️/🔽 icon for showing when it is open and closed)

i'm just sort of thinking out loud here but i think maybe i would rather deprecate o-expander (because i think o-toggle is closer to what we would want), the . so we'd talk to design about the idea of a disclosure component and then have something like:

<div data-o-component="o-disclosure">
  <button type="button o-toggle-button" aria-expanded="false" aria-controls="toggle-target">
    <span class="o-disclosure-icon">
      ▶️
    </span>
    Dogs
  </button>
  <div class="o-disclosure-target" id="toggle-target" aria-hidden="true">
    <ul>
      <li> border terrier
      <li> border collie
      <li> irish wolfhound
    </ul>
  </div>
</div>

which is not far off one of the current shape of o-toggle, and so maybe it would still be called o-toggle. but basically i'm suggesting we get a design and then we drop all of the different options for o-toggle and o-expander and allow only the exact shape of that design

notlee commented 2 years ago

nice, I agree let's propose to deprecate 👍 o-expander is another example of a super unhelpfully configurable component, like o-tooltip / o-overlay. let's do a bit more digging to find examples of ui built with toggle/expander and review with the design team

notlee commented 2 years ago

@JakeChampion get outa here it's a nice day today! 💛

chee commented 2 years ago

in addition, the way o-toggle is written made it difficult to add to Storybook too as mentioned in #719

chee commented 2 years ago

o-toggle is currently used in a couple of ways, but it is also not used very often.

a disclosure element

a way of toggling advanced controls section for a form

um

so

i think that's all of the uses. it's really not many. i think we should drop o-toggle. though, there may be a benefit in having a less complex library like this, that just takes care of the accessibility concerns: https://github.com/Financial-Times/origami/pull/719