origami icon indicating copy to clipboard operation
origami copied to clipboard

Deprecate o-toggle?

Open notlee opened this issue 6 years ago • 6 comments

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 avatar Sep 30 '19 17:09 notlee

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"

notlee avatar Aug 10 '20 09:08 notlee

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:

  • label when collapsed
  • label when expanded
  • a heading/summary (optional)
  • child content

a toggle looks like:

  • label for the button
  • child content

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

chee avatar Apr 22 '22 10:04 chee

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 avatar Apr 22 '22 11:04 notlee

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

notlee avatar Apr 22 '22 11:04 notlee

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

chee avatar Jun 13 '22 15:06 chee

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

a disclosure element

  • https://github.com/Financial-Times/ft-creative-ss3/blob/1ced034f0c380626bf4026685fa4e876b0dd1701/themes/base/templates/blocks/DisclaimerBlock.ss
  • https://github.com/Financial-Times/ez-bdp/blob/253d78f6619055b6454633d7b8e3b536d7f3a133/src/FT/NEDBundle/Resources/views/full/event/content.html.twig#L79-L85 (a disclosure? but the button is inside the section that is being toggled) (lots of examples of this use case on this repo)
  • https://github.com/Financial-Times/newspaper-control-centre/blob/edda4bf16503c5131dde272994eb0785c61ff97f/views/suspensions/index.html#L21-L27

a way of toggling advanced controls section for a form

  • https://github.com/Financial-Times/risk-register/blob/be393a258e9d5967b41a698c85e253ec287bc2cf/server/templates/HomePage.jsx#L33-L40
  • https://github.com/Financial-Times/ip-gdpr-hub-client/blob/bc30923cf5b01ecfe503fa503341c12c319ab8ac/views/new-request.html (this one is kind of disclosure, but in a form)
  • https://github.com/Financial-Times/biz-ops-admin/blob/da07b7b9858520da799e179ba559cd3ba3b5ff43/src/templates/report-page.jsx#L137-L151
  • https://github.com/Financial-Times/tech-hub/blob/a3dfbf175302395eecfe27c5430594990a7258b0/app/server/templates/technologyLifecycleTrackerPage.jsx#L468-L475

um

  • https://github.com/Financial-Times/ez-bdp/blob/253d78f6619055b6454633d7b8e3b536d7f3a133/src/FT/NEDBundle/Resources/views/page/header/main.html.twig#L7 (toggling body)

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

chee avatar Jun 13 '22 15:06 chee