Material-ui: [ToggleButton] docs: missing "size" property in the API docs

[croraf]

I'm checking current API documentation for ToggleButton https://material-ui.com/api/toggle-button/, and I don't see the size prop being mentioned there. But, the size property does affect the size of the ToggleButton. (I'm using the ToggleButton as a standalone though, not within the ToggleButtonGroup.)

Exploring the code for 4.11.0 I see this:
https://github.com/mui-org/material-ui/blob/v4.11.0/packages/material-ui-lab/src/ToggleButton/ToggleButton.js#L78
https://github.com/mui-org/material-ui/blob/v4.11.0/packages/material-ui-lab/src/ToggleButton/ToggleButton.js#L159-L162

And in the next branch this:

Does this mean the property is intentionally missing from the API, as it is not intended to be used by the users, so that it is only used internally by the enclosing ToggleButtonGroup component?

• [ X] The issue is present in the latest release.
• [ X] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Expected Behavior 🤔

Steps to Reproduce 🕹

Steps:

1.
2.
3.
4.

Context 🔦

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.11.0 |
| React | |
| Browser | |
| TypeScript | |
| etc. | |

[oliviertassinari]

@croraf Thanks for the report. In https://material-ui.com/components/toggle-button/#standalone-toggle-button, we have an example with a standalone ToggleButton component. It seems that we miss the documentation on the prop. What do you think about this diff?

diff --git a/packages/material-ui-lab/src/ToggleButton/ToggleButton.d.ts b/packages/material-ui-lab/src/ToggleButton/ToggleButton.d.ts
index cc68b0a89..d13483c5e 100644
--- a/packages/material-ui-lab/src/ToggleButton/ToggleButton.d.ts
+++ b/packages/material-ui-lab/src/ToggleButton/ToggleButton.d.ts
@@ -22,6 +22,11 @@ export type ToggleButtonTypeMap<
      * If `true`, the button will be rendered in an active state.
      */
     selected?: boolean;
+    /**
+     * The size of the button.
+     * The prop defaults to the value injected by the parent ToggleButtonGroup component, via cloneElement.
+     */
+    size?: 'small' | 'medium' | 'large';
     /**
      * The value to associate with the button when selected in a
      * ToggleButtonGroup.

If the format is approved, we could then mention in the other component how the parent influences the default props, e.g. with the stepper and inputs:

diff --git a/packages/material-ui/src/Step/Step.d.ts b/packages/material-ui/src/Step/Step.d.ts
index e89f479a7..bc03cf891 100644
--- a/packages/material-ui/src/Step/Step.d.ts
+++ b/packages/material-ui/src/Step/Step.d.ts
@@ -26,10 +26,12 @@ export interface StepProps
   expanded?: boolean;
   /**
    * The position of the step.
+   * The prop defaults to the value injected by the parent Stepper component, via cloneElement.
    */
   index?: number;
   /**
    * If `true`, the Step will be displayed as rendered last.
+   * The prop defaults to the value injected by the parent Stepper component, via cloneElement.
    */
   last?: boolean;
 }
diff --git a/packages/material-ui/src/InputBase/InputBase.d.ts b/packages/material-ui/src/InputBase/InputBase.d.ts
index 3373155e0..7745c1950 100644
--- a/packages/material-ui/src/InputBase/InputBase.d.ts
+++ b/packages/material-ui/src/InputBase/InputBase.d.ts
@@ -66,8 +66,8 @@ export interface InputBaseProps
    */
   inputRef?: React.Ref<any>;
   /**
-   * If `dense`, will adjust vertical spacing. This is normally obtained via context from
-   * FormControl.
+   * If `dense`, will adjust vertical spacing.
+   * The prop defaults to the value injected by the parent FormControl component, via context.
    */
   margin?: 'dense' | 'none';
   /**

[croraf]

I don't know. Are you sure that this prop should be present in the API docs?

Seems to me (as per what I explained in my first comment) is that the documentation for the prop is intentionally hidden?

[oliviertassinari]

Why should it be hidden?

[croraf]

My theory was that this property should be used only from the encompassing ToggleButtonGroup component. And the theory was backed by the usage of explicit @ignore in the 4.11.0 code, and explicit //eslint-disable-next-line react/prop-types in the next code.

Also while we are at it, its onChange and onClick props are ignored in a similar manner. So we should address them also.

[oliviertassinari]

• onChange is ignored because only make sense for the ToggleButtonGroup
• onClick is ignored because it's a native HTML attribute

I think that size should be documented because, when the component is used standalone, it's valuable.

[croraf]

Regarding size, I agree. But someone explicitly removed size from the propTypes and explicitly excluded eslint/prop-types checking on it.

Regarding onChange and onClick. I have to think about why both onChange and onClick exist, and why does onChange not make sense in standalone, and if it does not, how to handle showing it in API. I'm checking https://github.com/mui-org/material-ui/blob/next/packages/material-ui-lab/src/ToggleButton/ToggleButton.js right now and it is a spaghetti logic. The onClick is acually not passed to the child components.

[oliviertassinari]

@croraf The latest change was in #21687 with @eps1lon, however, the size prop was already ignored before that. In any case, I think that it was before we made a standalone ToggleButton a viable use case.

[croraf]

And what about onClick and onChange. In the code you can see that both onClick and onChange are manipulated in the ToggleButton itself, and not just transparently forwarded to the ButtonBase. I don't have time right now to explore in details what happens there, but from that fact alone it seems these 2 props should be in the API? Perhaps prefixed with underscore or something?

Should I open a new issue for these 2 props?

[eps1lon]

The prop defaults to the value injected by the parent ToggleButtonGroup component, via cloneElement.

Whe should remove mention of any implementation specifics. If this is important for the usage (interface) then describe that instead of concrete methods used.

[oliviertassinari]

@eps1lon Sounds great, it will avoid issues down the road if we want to refactor.

So same proposal but without via cloneElement and via context.

[zenje]

Hey, as a first issue, can I update ToggleButton docs to include the size prop? And from the above, should the Step and InputBase components docs also be updated as part of this issue? Thanks!

[oliviertassinari]

Hey, as a first issue, can I update ToggleButton docs to include the size prop?

@zenje It would be great :)

should the Step and InputBase components docs also be updated as part of this issue?

I think that it should be done separately.

[oliviertassinari]

@zenje Would you like to work on the second part? We could udpate the description of the props to explain where the states are inherited from https://github.com/mui-org/material-ui/issues/21976#issuecomment-664981661.

[zenje]

Sure @oliviertassinari! I can submit another PR to update the Step and InputBase docs.

[oliviertassinari]

@zenje These two components will be a great starting point :). It will probably require exploration in the codebase to find all the others. I would expect 10+ props like this.