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

Components: display a chevron icon in Item #36653

Open
Tracked by #34709
ciampo opened this issue Nov 19, 2021 · 4 comments
Open
Tracked by #34709

Components: display a chevron icon in Item #36653

ciampo opened this issue Nov 19, 2021 · 4 comments
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@ciampo
Copy link
Contributor

ciampo commented Nov 19, 2021

There are a few ways we could go about displaying a chevron icon in the Item component.

Currently, the Item component is a very generic and un-opinionated component, that doesn't care about its content — this makes it highly composable and reusable across many interfaces. We need to make sure that, whatever approach we end up taking, we don't make the Item component too opinionated or specific towards one use case.

Approaches

I though about a few approaches that we could take:

Approach 1: More Storybook examples

We could add a few more examples in Storybook, without touching the component's source code (approach taken in #36654 )

This approach has the advantage of leaving the Item component as-is, keeping it simple. Any developer in need of displaying a chevron icon could copy that example and use it in their own app.

Approach 2: Add functionality to <Item />

Another approach would be to make changes to the Item's component itself. We could borrow from UIKit's UITableViewCell and introduce the concept of Accessory Views, and allow each item to have a "start" and an "end" accessory view.

Screenshot 2021-11-19 at 14 38 34

This would definitely help "normalising" the UI/UX around this scenario, but would leave more questions open:

  • technically, the easiest way to implement this would be by adding 2 render props (one per accessory view). Would that be a good idea, or should we look at alternative approaches? (e.g slots)
  • What layout constraints should there be on the accessory views? For example, we could be very un-opinionated and just setup a simple flexbox layout to avoid any further issues)
  • To which extend would we address RTL support? It would be easy enough to swap start/end accessory views, but probably we should let the developer passing the Accessory View's content handle more specific RTL behaviour (e.g. flipping the chevron icon)

Approach 3: create a new, higher-level component

Finally, as an alternative to modifying the Item component directly, we should consider creating and exporting a new, more specialised component build on top of Item.

We would need to decide:

  • how specialised this component would be. E.g., should be similar to the Storybook example in approach 1 (i.e. very specific about adding a chevron icon), or should it be more generic, like approach 2 ?
  • Where would this component live? Would it make sense for it to be in the same folder with ItemGroup and Item ? And what should it be called?
  • we would probably have to answer many of the same questions listed in approach 2

Other considerations

The pattern that we decide to adopt here may also inform future decisions about how to best add high-level functionality to a component.

@ciampo ciampo added [Feature] Component System WordPress component system [Package] Components /packages/components labels Nov 19, 2021
@ciampo
Copy link
Contributor Author

ciampo commented Nov 19, 2021

Hey @griffbrad @mirka @diegohaz @jasmussen @mtias , could you please have a look at this issue and share your thoughts?

In the meantime, I've also opened #36654 to illustrate Approach 1

Thank you!

@mirka
Copy link
Member

mirka commented Nov 22, 2021

I'm leaning toward Approach 1 for now, and have a heavy reluctance for Approach 2.

When we are ready to commit to some specific usage patterns, I think higher-level components like Approach 3 are the way to go. By that point we'll have a better idea of what consumers need, or are struggling with.

The "accessory" naming is interesting. I like how generic it is, but it's also worth noting that we already have some components that use the terms "prefix" and "suffix" to denote things like these.

@ciampo
Copy link
Contributor Author

ciampo commented Nov 22, 2021

I also personally agree with @mirka .

I also personally prefer Approach 1 over Approach 2. I personally like the simplicity of the Item component, which lets its consumer define the layout of its children without any constraints.

I like Approach 3 too, but we'd need to first get a good feeling for the right balance to strike when creating higher-level components.

The "accessory" naming is interesting. I like how generic it is, but it's also worth noting that we already have some components that use the terms "prefix" and "suffix" to denote things like these.

This is great feedback. Adopting coherent naming conventions is going to be very important.

@ciampo
Copy link
Contributor Author

ciampo commented Nov 23, 2021

Update: The PR illustrating Approach 1 has been merged.

This doesn't preclude us from iterating with other approaches later on (especially with the higher-order components proposed in Approach 3).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants