Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[RFC] Alternatives to Awkward Variant Modifiers #2

Closed
dvdzkwsk opened this issue May 24, 2018 · 6 comments
Closed

[RFC] Alternatives to Awkward Variant Modifiers #2

dvdzkwsk opened this issue May 24, 2018 · 6 comments

Comments

@dvdzkwsk
Copy link
Member

dvdzkwsk commented May 24, 2018

Feature Request

Problem description

In Semantic UI V1 we supported modifiers/variants such as:

<Button basic />        // this is cool
<Button basic="very" /> // this is not

The problem with this is that it's extremely unintuitive, reads awkwardly, and overloads what should be a boolean or otherwise mutually exclusive property (basic).

Proposed solution

This issue is designed to encourage conversation around alternatives to this problem. One simple idea would be to support something like:

<Button basic />
<Button veryBasic />

Better ideas are encouraged.

@brianespinosa
Copy link
Member

brianespinosa commented Jun 6, 2018

I worry that this proposal means that components will have significantly more props on them. I completely agree with your initial suggestion though that the implicit boolean versus string is unintuitive.

What if we did something like basic="default" for the default implementation for a basic view and switched out the "default" string with whatever the modifier is like "very". Maybe "default" is not the correct string here either. Other possibilities might be "initial".

@dvdzkwsk
Copy link
Member Author

dvdzkwsk commented Jun 6, 2018

Definitely agree that an explosion of props is not ideal. That said, I'm not convinced that a default string is the answer, since it would be extremely difficult to come up with a value that is as universal and straightforward as a boolean, especially since the shorthand for true is so convenient in React: <Button basic />.

Taking a step back, it seems like there's a meaningful distinction between the two approaches. With basic|veryBasic, it's implied that each of the props act on the component level (i.e., the component is either basic or veryBasic), and they are autonomous modifiers. With basic and basic="very" (or the default string, in your case), "very" acts as a modifier on basic instead.

This seems like a core distinction, and may help us figure out what approach to take. Is it important that we support varying magnitudes of a property (or certain properties)? If so, veryBasic should be eliminated and we should pursue more intuitive ways of modifying basic. In this case your suggestion is a good start. Perhaps @levithomason can share his thoughts on this.


P.S.

For the sake of argument, in my opinion basic is not something that should accept modifiers. Something is either basic or it isn't. Given this belief, a prop such as veryBasic makes sense, though the naming could be better (plain, minimal, etc.). Other properties may very well have modifiers, though.

@brianespinosa
Copy link
Member

For the sake of argument, in my opinion basic is not something that should accept modifiers. Something is either basic or it isn't. Given this belief, a prop such as veryBasic makes sense, though the naming could be better (plain, minimal, etc.). Other properties may very well have modifiers, though.

Agreed here.

I think we may want to review all SUIR cases where a prop we have modifying props that are enumerated as either true or an expected set of strings. Looking at them all together might shake out a better baseline string. Or different solution.

@levithomason
Copy link
Member

Note, the result of this conversation will be a direct update to the API guidelines for contributing:

https://github.com/levithomason/stardust/blob/master/.github/CONTRIBUTING.md#api

It is worth reviewing that ^ to understand how and why we have the 4 types of props in the library currently.

@levithomason
Copy link
Member

Additionally, I was working on an "API Explorer" which I think I'll return to. I want to analyze the library's current components and props.

We should know how many different props we have, how many conventions there are, which props are global, which are isolated to a single component, etc. This will enable us to make data driven decisions on reducing API surface moving forward. Generating this information per PR will also enable us to review API impacts on a per PR basis.

@dvdzkwsk
Copy link
Member Author

Moved to microsoft/fluent-ui-react#2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants