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

Enforce accessibility best practices by making some parameters mandatory #69

Closed
GlynnPhillips opened this issue Jul 7, 2020 · 4 comments
Assignees
Labels
meta Relates to an Origami meta proposal A proposed change which requires approval or discussion
Projects

Comments

@GlynnPhillips
Copy link
Contributor

What

Enforce accessibility best practices by making some component parameters mandatory.

Details

We often see issues raised in the DAC accessibility report that are caused by people using components and not using some of the options designed to provide extra context. A example would be the closeButtonLabel in o-banner

In the past we have fixed some of these by creating a catch all default but these often fall short as they are missing the context required from the parent application.

We could throw an error in a similar way to the errors thrown for not setting a theme https://github.com/Financial-Times/o-banner/blob/master/src/js/banner.js#L52

As this would need to be a new breaking changing we would want to review all the options along with any other accessibility options we don't currently have but would be nice to add at the same time.

@origamiserviceuser origamiserviceuser added this to incoming in Origami ✨ Jul 7, 2020
@github-actions github-actions bot added the meta Relates to an Origami meta label Jul 7, 2020
@JakeChampion
Copy link
Contributor

Sorry for the slow reply on this!

I think this is a really good idea, thank you for raising it Glynn.

This looks like it can be rolled out on a component-by-component basis, using o-banner first as you have already pointed out it recommends setting at buttonLabel. With regards to o-banner, it has a default value for buttonLabel of "OK", I guess we would make it not have a default value and instead throw an error.

@chee chee added pattern proposal A proposed change which requires approval or discussion labels Dec 7, 2020
Origami ✨ automation moved this from incoming to complete Dec 7, 2020
@Financial-Times Financial-Times locked and limited conversation to collaborators Sep 8, 2021
@chee chee reopened this Sep 8, 2021
@Financial-Times Financial-Times unlocked this conversation Sep 8, 2021
@chee chee removed the pattern label Sep 17, 2021
@notlee
Copy link
Contributor

notlee commented Jan 20, 2022

Here's an example of us doing this (removing the data-o-tabs-disablefocus="true" option):
#571

There're lots of other places Origami options still enable poor accessibility practices, like the o-banner issue pointed out in above, and o-overlay, and o-tooltip...

@Financial-Times/origami-core Do you think we should close this issue? I'm not sure it's helpful unless we do an audit of all component permutations, which is a lot (too many). I think it's something the team are aware of and should improve as we complete audits and revamp older components.

@wheresrhys
Copy link
Contributor

I also vote to close this as (I guess?) the TSX templates will have these attributes and validation baked in. Therefore the golden path to achieving accessibility with origami becomes 'use the TSX templates' (+ a typed build step?), and saves having to craft bespoke validation rules in js

@notlee
Copy link
Contributor

notlee commented Mar 21, 2023

Cool, I'll close this. It'll take more than the new tsx templates, not least because they're optional, but that's one (non breaking) way we can enable teams to use Origami components in an accessible way. As we audit and rebuild components with fewer, opinionated usecases in mind, we can ensure it's simpler to use all variations accessibly. In the meantime we can rely on documentation as issues arise.

@notlee notlee closed this as completed Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Relates to an Origami meta proposal A proposed change which requires approval or discussion
Projects
Origami ✨
  
Done
Development

No branches or pull requests

6 participants