mui / mui-toolpad

Toolpad Studio: Low-code admin builder. Open-source and powered by MUI.
https://mui.com/toolpad/
MIT License
815 stars 206 forks source link

Make it more obvious when the sx prop is used #787

Open oliviertassinari opened 1 year ago

oliviertassinari commented 1 year ago

Duplicates

Latest version

Summary 💡

I'm reporting a small frustration I had. If you open https://master--toolpad.mui.com/_toolpad/app/cl42qt2jx01249xnr14xjce8b/editor/pages/cl6r0xb1f00003b6ghdh8e37l

Screenshot 2022-08-13 at 00 17 34

can you figure out why the text field is not full width? 😁

Solution: it's coming from:

{
  "width": 175
}

Examples 🌈

No response

Motivation 🔦

No response

Janpot commented 1 year ago

Also related to https://github.com/mui/mui-toolpad/issues/38

prakhargupta1 commented 1 year ago

User feedback: if I didn't know "sx" was a MUI thing, I'd see it on the sidebar and think "size?" So we should link the sx prop page here to help developers understand the concept better.

bytasv commented 1 year ago

Maybe also rename it in UI and call it style or something more human readable?

Janpot commented 1 year ago

So we should link the sx prop page here to help developers understand the concept better.

We are linking the sx prop page:

Screenshot 2023-01-11 at 10 26 50

There is a tracking issue to create a more human friendly version of it.

prakhargupta1 commented 1 year ago

Q1. Somehow the above helper text, remains longer than intended. Having it accessible through a ? icon would solve this problem

https://user-images.githubusercontent.com/92228082/214042574-db7b230c-aced-4174-91bb-bf283ecb5c18.mov

Q2. Like the way we show 2 props already setup for the container component, is it possible to show it for all components? We can show the default styling values. It will help the user quickly play around some of them and understand how styling works here.

bytasv commented 1 year ago

Q2. Like the way we show 2 props already setup for the container component, is it possible to show it for all components?

container component has these defaults defined by us, so it's not a container styling props, rather it's overrides. We can show these props for other components too, but first they have to be defined individually

prakhargupta1 commented 1 year ago

container component has these defaults defined by us, so it's not a container styling props, rather it's overrides. We can show these props for other components too, but first they have to be defined individually

Understood. I'll try and define these material design defaults and create as issue for it later.

gerdadesign commented 1 year ago

It seems like there's a couple factors here. For the scope of this issue, below is a suggestion for how to visually display when the sx prop is being used.

Default

default

Hovering (tooltip)

hover

SX prop is being used

enabled

Adding data binding automatically means that SX prop is being used

databinding

Janpot commented 1 year ago

A switch without immediate feedback feels a bit against the MD guidelines. It's a bit counterintuitive to me when clicking a switch opens a dialog. (Not that we have to be religious about this as far as I'm concerned).

Could there be alternatives similar to how we did the binding button? inactive/active state + different icon?

oliviertassinari commented 1 year ago

It's a bit counterintuitive to me when clicking a switch opens a dialog.

Agree, I think that we should avoid this. For example, to edit an sx prop, does it mean we have to disable and reenable it?

I think that this GitHub issue is solved as soon as we have a visual change between sx empty, and sx not empty, e.g.:

Screenshot 2023-01-29 at 18 59 04

where the label is "Edit" or "Add" depending on the state.


@gerdadesign something is off with the upload of the screenshots to GitHub. We got:

default

but we should have:

I think that it's important so we can be closer to how it will feel like. I can't really visualize/project myself using this UI in the former image, it's easier in the latter. Normally, with macOS screenshots, GitHub can extract the image resolution and add the right width automatically.

Janpot commented 1 year ago

Also want to add, this doesn't necessarily have to be a single line/button. e.g We could think up a larger component containing 5 nested editors for the 5 most used css properties, and a button to open an editor in a modal/popover for all the other properties.

prakhargupta1 commented 1 year ago

I agree with Olivier's point that separating title and action will remove the confusion about whether the sx prop is being used or not. About how to show the action Add/Edit, I was thinking if we should show an edit (pencil) icon. Benefits:

  1. It will be consistent with the binding icon.
  2. States like used/error/not used can be shown like we show for binding icon.
  3. The same pattern can be extended to other component props like a) Datagrid: rows, selection b) Select: options

Also, we need to ensure that a pattern is followed for all possible combinations in the right sidebar. We have text input, drop-down, buttons etc. We have a separate task for this: https://github.com/mui/mui-toolpad/issues/1488

apedroferreira commented 1 year ago

Plus 1 for feeling like a switch is not the best solution, it feels more appropriate for something that can only be true/false Also yes, it might be a good idea if every control in the right panel that opens a modal follows the same type of pattern.

oliviertassinari commented 1 year ago

On the topic of the tooltip helpers:

Screenshot 2023-02-23 at 14 29 43 Screenshot 2023-02-23 at 12 17 57 Screenshot 2023-02-23 at 12 17 52
Janpot commented 1 year ago

On the topic of the tooltip helpers:

I think a hybrid of those could make sense. e.g. either

I also like that both of these examples are quite discoverable with a sort of ⓘ icon. I feel like that's missing the most from Toolpad at the moment. The current components aren't very dense and the label doesn't really allow for adding an icon like that.

gerdadesign commented 1 year ago

Question about this SX prop within the context of the new direction. If our target user is a backend dev, it does not seem that they would know how to use the SX prop, nor want to read through the documentation to understand how to use it. Also, would it be possible to bind data to each individual property within the SX prop or only for the SX prop itself? I'm curious what makes most sense from the options I see here:

1. Button Similar to the existing experience — when nothing is bound, selecting "add" would open either A. a dialog OR B. bring you to a specific section in your code editor (if going, with this option 1, does A or B make more sense?) If you already have something bound, it would changed Add → Edit.

Toolpad_SX_button

2. Editor Instead of a pop-up dialog, the editor is directly accessible without leaving the panel. This would make it faster to access, but could get crowded if there is a lot of code.

Toolpad_SX_editor

3. Exposed props Assuming most backend devs don't know about the SX prop nor want to learn it, should the most common/relevant properties be more exposed? plus an option to edit via code? This would allow for greater clarity of what you can adjust, but could feel limited (although we would always provide an escape hatch). This primarily necessitates the question of is it possible to bind each property (margin, padding, color, etc) or do you bind the entire sx prop overall?

Toolpad_SX_exposedprops

Or, if there is a different way of showing this that makes more sense, eager to hear your ideas.

Janpot commented 1 year ago

Personally, 3. is what I would advocate for going forward. Some notes:

gerdadesign commented 1 year ago

Yeah, I was kind of wishing for something like this in the current iteration, at the very least to help with formatting. I may know I'm looking for border-color, or text-decoration but used to the CSS formatting, so finding it in JSON takes extra time (just because I don't know).

If we go with something more freeform like this, could we provide an autocomplete (in JSON format?) with all available properties? Or only properties for which we want to support binding? Can we specify which we do/do not allow?

Janpot commented 1 year ago

If we go with something more freeform like this, could we provide an autocomplete (in JSON format?) with all available properties? Or only properties for which we want to support binding? Can we specify which we do/do not allow?

Where my mind was when I wrote that was to iterate further on the component we use for parameters or url queries:

Screenshot 2023-03-15 at 13 03 43

a. To make this more visually coherent, e.g. stick them together, remove the labels,... b. To allow these text inputs to be autocompletes like https://mui.com/material-ui/react-autocomplete/. We should be able restrict the options available in those.

But alternatively we could look into adding autocomplete in the JSON editor itself. It currently is possible to define a schema for the JSON editor: See https://github.com/mui/mui-toolpad/pull/1753

Also, shall we split off the issues around discoverability of inline documentation into a separate issue? (https://github.com/mui/mui-toolpad/issues/787#issuecomment-1441901855)

apedroferreira commented 1 year ago

I'm not sure which is best from something like option 1 or more like option 3... The sx properties that people might want to customize could also vary a lot depending on the component. But I really like the approach of having a list of name/value pairs that we can add to / delete from, and where all the possible property names are suggested, probably with an autocomplete since there are so many options.

prakhargupta1 commented 1 year ago

I think 3. makes sense as this will mean lesser code writing in the UI editor resulting in better UX than having autocomplete in an editor. Choosing from a list of drop-down options, typing input fields would make app building faster.

Also, for a more detailed component, the user anyway would have the option to create a custom component that can have inline sx/styling. So either pro-code or low-code.

But the dev tools vide looks good too, all props are getting validated inline. If maintaining this can be faster, something like this in our JSON editor might be a good way to start and keep UX clean, minimal.