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

Whitelisting certain props? #22

Closed
shawnbot opened this issue May 18, 2018 · 2 comments
Closed

Whitelisting certain props? #22

shawnbot opened this issue May 18, 2018 · 2 comments

Comments

@shawnbot
Copy link
Contributor

This week's work has given us an opportunity to mull over whether components should be "strict" in which props they allow users to pass — or, as an implementation detail, which ones they actually respect. I can think of a couple that we may want to whitelist for some, if not all, Primer components:

  1. id could people to provide deep links without having to wrap another element around their content.
  2. hidden would provide a more systematic way to show and hide things.
  3. Any aria- attribute, because a11y.
  4. Any data- attribute, to provide hooks for JS behaviors implemented by container components.

Thoughts?

@shawnbot
Copy link
Contributor Author

Because we're talking about implementing display more generally, I think we should also discuss having hidden override that value so that:

expect(render(<FlexContainer hidden />)).toEqual(render(<div hidden />))
expect(render(<Block display="flex" hidden />)).toEqual(render(<Block hidden />))

in other words, something like:

// props.js
export const mapHidden = ({display, hidden, ...rest}) => {
  // don't pass the display prop if hidden is true
  return hidden === true ? {hidden, ...rest} : {display, ...rest}
}

@shawnbot shawnbot changed the title Whitelisting certain props Whitelisting certain props? Jul 23, 2018
@shawnbot
Copy link
Contributor Author

shawnbot commented Aug 8, 2018

The strategy with system-components is actually to blacklist props that shouldn't be forwarded, so this is OBE.

@shawnbot shawnbot closed this as completed Aug 8, 2018
Primer Components release tracking 📋 automation moved this from Backlog to 1.0.0-beta Completed Aug 8, 2018
@emplums emplums moved this from 2.0.0-beta Completed to 🚢Published in Primer Components release tracking 📋 Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

1 participant