Mobile components are diverging too much

[koke]

I have growing concerns about several issues with our native versions of some components that I've spotted over several PRs recently. I can list the details later, but there are two main ones: Cells and Panels.

Cells: not every control is one

@wordpress/components is about creating a set of UI elements that you can reuse anywhere. So far, every *Control element that we have implemented for native, uses a variant of Cell.

I understand this is because our current use case is putting these in a BottomSheet, but there's nothing about a TextControl that says it will be used in a BottomSheet. For instance, the rss block uses a TextControl within the edit interface. Or the contact forms in Jetpack.

TextControl is a particularly interesting example because it's not even what the control is supposed to be: the native variant is more of a Button than a text input. But we also have SelectControl (uses PickerCell), ToggleControl (uses SwitchCell), and RangeControl (which seems like it skipped adding a native variant of the existing control and used the RangeCell directly #17282 #17896).

We need to find a better way to implement these components so they work anywhere a developer might insert them. And then, figure out how to style them properly when used in a bottom sheet.

Panels, what are they?

The README for Panel has this to say:

Panels expand and collapse multiple sections of content

However, we don’t collapse the settings sections in mobile. Maybe we’ll want to do that at some point in the future, when we have more settings available, but it seemed like a better UX to not do that.

But we still use Panel, even though it’s function is to provide a title and spacing so a group of cells looks like a “section. Then we wanted some buttons in the settings outside any group, so we ended up with PanelActions in #17703, even though those don’t have anything to do with the original meaning of Panel.

Semantics vs. Looks

I think both problems reflect the same issue. Gutenberg has created several abstractions that have worked generally well. But, understandably, many of those have ended up tied to how things would end up looking in the web. It didn’t make sense to add an extra level of abstraction if things were only meant to be displayed in one way, but now things are different.

I think we have several problems to solve:

1. We need to group things within InspectorControls. A InspectorControlsGroup would allow a better abstraction where the web can show collapsible panels and the apps can have regular sections without collapsing. If we needed a Button, we could just add one outside any group, and it would display as expected.
2. I mentioned this on the Card PR (Components: Add Card Component #17963), but I’d want to explore how do we define styles for components within another container component. I think we should be able to put a Button inside a InspectorControlsGroup and have it automatically get the right style (without needing an InspectorControlsButton). This has been done before on the web via CSS, but if we are exploring styled components and cross-platform solutions, I would like to solve this as well.
3. I think the right solution for controls in BottomSheet is in the previous point. Being able to define how RangeControl looks in a BottomSheet might be enough, but we could need newer abstractions to make those work, and that’s OK

/cc @WordPress/native-mobile

[mcsf]

cc @ItsJonQ, you may be interested in this.

[ItsJonQ]

@mcsf Thanks for the heads up!

@koke Awesome break down! And thank you for being so active with the discussions!

I’d want to explore how do we define styles for components within another container component...

I would love to as well! As an experiment, maybe we can explore how we can refactor a single component, maybe Panel?, that can work well for web + native. With a balanced component API that's flexible and opinionated.

The tricky thing is, (for me), I don't know of all the ways we'd want something like Panel to render and function for both platforms. It's like we're building as we go, haha.

Since we're starting to leverage Storybook more for web UI, I think it'll be handy to set it up for React Native as well.

I think I've chatted about React Native + Storybook briefly with @diegohaz.

I think the concerns you've raised is great!! <3

[mtias]

I completely agree. Any current interface should be considered ephemeral from the perspective of declaring the functionality of a block. The InspectorControls component is a good example because it purposely tried to avoid location references (as in: "Block Sidebar") under the assumption that whether the controls are displayed in a sidebar or somewhere else (a modal, the mobile bottom sheet, etc) is irrelevant to the block author and is subject to change.

Panel is one where the current interface has dictated too much of the inherent semantics of the components which is a mistake. The block author should be declaring which tools exist as "inspector tools" vs "toolbar tools". It's then up to the editor application to map the tools to the relevant interface elements (panels or something else) accordingly.

Having the mobile project running alongside is a great incentive to make sure the semantics are both general enough and meaningful enough, because we also don't want to create abstractions that end up being obscure.

InspectorControlsGroup kind of makes sense, but I'm not sure I understand what InspectorControlsButton would represent. Wouldn't a developer add a regular Button either to the root of InspectorControls or within InspectorControlsGroup?

[mcsf]

#16384 (comment) bears similar stakes around the semantics—physicality debate.

[koke]

Wouldn't a developer add a regular Button either to the root of InspectorControls or within InspectorControlsGroup?

@mtias I think that would be the ideal API, but is not clear to me how that would work in a CSS-in-JS solution yet

[jameslnewell]

In styled-components you can override styles in a very similar way to how we'd override styles in vanilla CSS, with similar pros and cons.

// button.js
export const Button = styled.button`
  color: green;
`;

// inspector-controls.js
import {Button} from './button.js';
export const InspectorControls = styled.div`
  ${Button} {
    color: red;
  }
`;

Though I don't believe this functionality is supported by emotion. /cc @diegohaz

I think the preferred approach is to use composition - the InspectorControlsButton approach.

// button.js
export const Button = styled.button`
  color: green;
`;

// inspector-controls.js
import {Button} from './button.js';
export const InspectorControls = styled.div``;
export const InspectorControlsButton = styled(Button)`color: red;`;

The semantics of a Button being a Button everywhere is nice, but, practically if the context of where a button is used dictates a slightly different usage, style or behaviour, then InspectorControlsButton is a necessary pattern - particularly if you throw types into the mix!

As a theoretical example, in InspectorControls we probably want to constrain the use of button props in order to maintain a consistent button sizing across all buttons in the group:

// we want consistent sizing
<InspectorControls size="large">
  <InspectorControls.Button>A</InspectorControls.Button>
  <InspectorControls.Button>B</InspectorControls.Button>
</InspectorControls>

// instead of this
<InspectorControls>
  <Button isSmall>A</Button>
  <Button isLarge>B</Button>
</InspectorControls>

There might be a bunch of other properties we might want to constrain on a button within InspectorControls e.g. we might not allow the isLink variation of buttons.

[koke]

That makes perfect sense to me, and it aligns with what I'm used to seeing in mobile. As long as InspectorControls.Button is implemented using Button and reusing the same behavior that we have for the rest of buttons, it makes sense to add a new type of component to constraint the possible configurations to those that make sense within InspectorControls.

Maybe I don't fully understand styled() yet, but in your example, since InspectorControlsButton = styled(Button), wouldn't it still be possible to add a <InspectorControlsButton isSmall/>?

[jameslnewell]

Maybe I don't fully understand styled() yet, but in your example, since InspectorControlsButton = styled(Button), wouldn't it still be possible to add a ?

Yep apologies, that was an over-simplified example!

Here's a an example where I've implemented the pattern in recently, using context to pass down shared props like size from the container component and limiting the set of props which can be set on the button itself.
https://github.com/salad-ui/components/blob/d3ee0b688bf01cc7e5954a9a7c0b556cd49e1123/packages/button/src/button-group/ButtonGroupButton.tsx

[jordesign]

@koke Just checking in to see whether this is still an issue for you in testing.

[koke]

@jordesign sorry, I haven't worked on gutenberg for the last few years, so I'm not sure if this is still an issue