Merge pull request #5132 from marmelab/revert-too-much-tabs

Revert Add scroll buttons to tabs when they go beyond the available space
marmelab · Aug 11, 2020 · 6a5d062 · 6a5d062
2 parents fdb449b + 7557bf2
commit 6a5d062
Show file tree

Hide file tree

Showing 8 changed files with 10 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,6 @@
 * Add ability to opt out of `sanitizeEmptyValues` in `<SimpleForm>` and `<TabbedForm>` ([5077](https://github.com/marmelab/react-admin/pull/5077)) ([Kmaschta](https://github.com/Kmaschta))
 * Add ability to make the `<SaveButton>` not disabled by default ([5002](https://github.com/marmelab/react-admin/pull/5002)) ([Luwangel](https://github.com/Luwangel))
 * Add ability to cutomize Add and Remove buttons in `<SimpleFormIterator>` ([4818](https://github.com/marmelab/react-admin/pull/4818)) ([manishsundriyal](https://github.com/manishsundriyal))
-* Add scroll buttons to Tabs when they go beyond the available space ([4538](https://github.com/marmelab/react-admin/pull/4538)) ([JulienMattiussi](https://github.com/JulienMattiussi))
 * Fix bad type for `useQuery` options (`onError` -> `onFailure`) ([5130](https://github.com/marmelab/react-admin/pull/5130)) ([fzaninotto](https://github.com/fzaninotto))
 * Fix `<ReferenceInput>` throws exception on custom pages ([5129](https://github.com/marmelab/react-admin/pull/5129)) ([fzaninotto](https://github.com/fzaninotto))
 * Fix `TypeError` when suggested element in `<AutocompleteInput>` is empty ([5125](https://github.com/marmelab/react-admin/pull/5125)) ([fzaninotto](https://github.com/fzaninotto))

diff --git a/docs/CreateEdit.md b/docs/CreateEdit.md
@@ -724,7 +724,6 @@ Here are all the props accepted by the `<TabbedForm>` component:
 * `save`: The function invoked when the form is submitted. This is passed automatically by `react-admin` when the form component is used inside `Create` and `Edit` components.
 * `saving`: A boolean indicating whether a save operation is ongoing. This is passed automatically by `react-admin` when the form component is used inside `Create` and `Edit` components.
 * [`warnWhenUnsavedChanges`](#warning-about-unsaved-changes)
-* `scrollable`: A boolean, `true` by default. `<TabbedForm>` uses Material UI scrollable buttons when there are too many tabs to display. If you prefer the screen to expand with the tabs, set it to `false`.
 * [`sanitizeEmptyValues`](#setting-empty-values-to-null)
 
 {% raw %}

diff --git a/docs/Show.md b/docs/Show.md
@@ -314,8 +314,6 @@ export const PostShow = (props) => (
 ```
 {% endraw %}
 
-**Tip**: By default, `<TabbedShowLayout>` use Material UI scrollable buttons when there are too many tabs to display. If you prefer the screen to expand with the tabs, add this prop: `scrollable={false}`.
-
 To style the tabs, the `<Tab>` component accepts two props:
 
 - `className` is passed to the tab *header*

diff --git a/packages/ra-ui-materialui/src/detail/TabbedShowLayout.js b/packages/ra-ui-materialui/src/detail/TabbedShowLayout.js
@@ -4,7 +4,6 @@ import PropTypes from 'prop-types';
 import Divider from '@material-ui/core/Divider';
 import { Route } from 'react-router-dom';
 import { makeStyles } from '@material-ui/core/styles';
-import classnames from 'classnames';
 import { useRouteMatch } from 'react-router-dom';
 import { escapePath } from 'ra-core';
 
@@ -16,7 +15,6 @@ const sanitizeRestProps = ({
     record,
     resource,
     basePath,
-    scrollable,
     version,
     initialValues,
     staticContext,
@@ -28,23 +26,10 @@ const sanitizeRestProps = ({
 const useStyles = makeStyles(
     theme => ({
         content: {
-            paddingTop: props =>
-                props.scrollable ? theme.spacing(7) : theme.spacing(1), // When using scrollable tabs, leave enough height for the tab content
+            paddingTop: theme.spacing(1),
             paddingLeft: theme.spacing(2),
             paddingRight: theme.spacing(2),
         },
-        scrollableDivider: {
-            marginTop: theme.spacing(6),
-            position: 'absolute',
-            width: '100%',
-        },
-        scrollableTabs: {
-            position: 'absolute',
-            width: '100%',
-        },
-        formRelative: {
-            position: 'relative',
-        },
     }),
     { name: 'RaTabbedShowLayout' }
 );
@@ -96,7 +81,6 @@ const TabbedShowLayout = props => {
         className,
         record,
         resource,
-        scrollable,
         version,
         value,
         tabs,
@@ -106,23 +90,11 @@ const TabbedShowLayout = props => {
 
     const classes = useStyles(props);
 
-    const scrollableProps = scrollable
-        ? { scrollable: true, scrollButtons: 'on', variant: 'scrollable' }
-        : {};
-
     return (
-        <div
-            className={classnames(className, classes.formRelative)}
-            key={version}
-            {...sanitizeRestProps(rest)}
-        >
-            {cloneElement(tabs, { classes, ...scrollableProps }, children)}
+        <div className={className} key={version} {...sanitizeRestProps(rest)}>
+            {cloneElement(tabs, {}, children)}
 
-            <Divider
-                className={classnames({
-                    [classes.scrollableDivider]: scrollable,
-                })}
-            />
+            <Divider />
             <div className={classes.content}>
                 {Children.map(children, (tab, index) =>
                     tab && isValidElement(tab) ? (
@@ -155,15 +127,13 @@ TabbedShowLayout.propTypes = {
     record: PropTypes.object,
     resource: PropTypes.string,
     basePath: PropTypes.string,
-    scrollable: PropTypes.bool,
     value: PropTypes.number,
     version: PropTypes.number,
     tabs: PropTypes.element,
 };
 
 TabbedShowLayout.defaultProps = {
     tabs: <TabbedShowLayoutTabs />,
-    scrollable: true,
 };
 
 export default TabbedShowLayout;
diff --git a/packages/ra-ui-materialui/src/detail/TabbedShowLayoutTabs.js b/packages/ra-ui-materialui/src/detail/TabbedShowLayoutTabs.js
@@ -9,25 +9,16 @@ export const getTabFullPath = (tab, index, baseUrl) =>
         tab.props.path ? `/${tab.props.path}` : index > 0 ? `/${index}` : ''
     }`;
 
-const TabbedShowLayoutTabs = ({ classes, children, scrollable, ...rest }) => {
+const TabbedShowLayoutTabs = ({ children, ...rest }) => {
     const location = useLocation();
     const match = useRouteMatch();
 
     // The location pathname will contain the page path including the current tab path
     // so we can use it as a way to determine the current tab
     const value = location.pathname;
 
-    const scrollableProps = scrollable
-        ? { className: classes.scrollableTabs }
-        : {};
-
     return (
-        <Tabs
-            indicatorColor="primary"
-            value={value}
-            {...scrollableProps}
-            {...rest}
-        >
+        <Tabs indicatorColor="primary" value={value} {...rest}>
             {Children.map(children, (tab, index) => {
                 if (!tab || !isValidElement(tab)) return null;
                 // Builds the full tab tab which is the concatenation of the last matched route in the
@@ -46,7 +37,6 @@ const TabbedShowLayoutTabs = ({ classes, children, scrollable, ...rest }) => {
 };
 
 TabbedShowLayoutTabs.propTypes = {
-    classes: PropTypes.object,
     children: PropTypes.node,
 };
 

diff --git a/packages/ra-ui-materialui/src/form/TabbedForm.js b/packages/ra-ui-materialui/src/form/TabbedForm.js
@@ -74,7 +74,6 @@ import TabbedFormTabs, { getTabFullPath } from './TabbedFormTabs';
  * @prop {ReactElement} toolbar The element displayed at the bottom of the form, containing the SaveButton
  * @prop {string} variant Apply variant to all inputs. Possible values are 'standard', 'outlined', and 'filled' (default)
  * @prop {string} margin Apply variant to all inputs. Possible values are 'none', 'normal', and 'dense' (default)
- * @prop {boolean} scrollable The tabs become scrollable when they extend beyond the witdh of the form
  * @prop {boolean} sanitizeEmptyValues Wether or not deleted record attributes should be recreated with a `null` value (default: true)
  *
  * @param {Prop} props
@@ -101,31 +100,17 @@ TabbedForm.propTypes = {
     submitOnEnter: PropTypes.bool,
     undoable: PropTypes.bool,
     validate: PropTypes.func,
-    scrollable: PropTypes.bool,
     sanitizeEmptyValues: PropTypes.bool,
 };
 
 const useStyles = makeStyles(
     theme => ({
         errorTabButton: { color: theme.palette.error.main },
         content: {
-            paddingTop: props =>
-                props.scrollable ? theme.spacing(7) : theme.spacing(1), // When using scrollable tabs, leave enough height for the tab content
+            paddingTop: theme.spacing(1),
             paddingLeft: theme.spacing(2),
             paddingRight: theme.spacing(2),
         },
-        scrollableDivider: {
-            marginTop: theme.spacing(6),
-            position: 'absolute',
-            width: '100%',
-        },
-        scrollableTabs: {
-            position: 'absolute',
-            width: '100%',
-        },
-        formRelative: {
-            position: 'relative',
-        },
     }),
     { name: 'RaTabbedForm' }
 );
@@ -145,7 +130,6 @@ export const TabbedFormView = props => {
         redirect: defaultRedirect,
         resource,
         saving,
-        scrollable = true,
         setRedirect,
         submitOnEnter,
         tabs,
@@ -163,34 +147,21 @@ export const TabbedFormView = props => {
     const location = useLocation();
 
     const url = match ? match.url : location.pathname;
-    const scrollableProps = scrollable
-        ? { scrollable: true, scrollButtons: 'on', variant: 'scrollable' }
-        : {};
     return (
         <form
-            className={classnames(
-                'tabbed-form',
-                className,
-                classes.formRelative
-            )}
+            className={classnames('tabbed-form', className)}
             {...sanitizeRestProps(rest)}
         >
             {React.cloneElement(
                 tabs,
                 {
                     classes,
                     url,
-                    title: 'FormTabRow',
                     tabsWithErrors,
-                    ...scrollableProps,
                 },
                 children
             )}
-            <Divider
-                className={classnames({
-                    [classes.scrollableDivider]: scrollable,
-                })}
-            />
+            <Divider />
             <div className={classes.content}>
                 {/* All tabs are rendered (not only the one in focus), to allow validation
                 on tabs not in focus. The tabs receive a `hidden` property, which they'll
@@ -316,7 +287,6 @@ const sanitizeRestProps = ({
     reset,
     resetSection,
     save,
-    scrollable,
     staticContext,
     submit,
     submitAsSideEffect,

diff --git a/packages/ra-ui-materialui/src/form/TabbedForm.spec.js b/packages/ra-ui-materialui/src/form/TabbedForm.spec.js
@@ -92,63 +92,4 @@ describe('<TabbedForm />', () => {
             expect(tabs).toEqual(['tab1', 'tab3', 'tab4']);
         });
     });
-
-    it('should have scroll buttons when too much Tabs />', () => {
-        const { queryByLabelText, queryByTitle } = renderWithRedux(
-            <MemoryRouter initialEntries={['/']}>
-                <TabbedForm
-                    style={{
-                        width: 200,
-                        maxWidth: 200,
-                    }}
-                >
-                    <FormTab label="A Useful Tab 1" />
-                    <FormTab label="A Useful Tab 2" />
-                    <FormTab label="A Useful Tab 3" />
-                    <FormTab label="A Useful Tab 4" />
-                    <FormTab label="A Useful Tab 5" />
-                    <FormTab label="A Useful Tab 6" />
-                    <FormTab label="A Useful Tab 7" />
-                </TabbedForm>
-            </MemoryRouter>
-        );
-
-        const tabs = queryByLabelText('Form-tabs');
-
-        expect(tabs.children).toHaveLength(7);
-
-        const tabRow = queryByTitle('FormTabRow');
-
-        expect(tabRow.children).toHaveLength(4);
-    });
-
-    it('should not have scroll buttons when too much Tabs />', () => {
-        const { queryByLabelText, queryByTitle } = renderWithRedux(
-            <MemoryRouter initialEntries={['/']}>
-                <TabbedForm
-                    style={{
-                        width: 200,
-                        maxWidth: 200,
-                    }}
-                    scrollable={false}
-                >
-                    <FormTab label="A Useful Tab 1" />
-                    <FormTab label="A Useful Tab 2" />
-                    <FormTab label="A Useful Tab 3" />
-                    <FormTab label="A Useful Tab 4" />
-                    <FormTab label="A Useful Tab 5" />
-                    <FormTab label="A Useful Tab 6" />
-                    <FormTab label="A Useful Tab 7" />
-                </TabbedForm>
-            </MemoryRouter>
-        );
-
-        const tabs = queryByLabelText('Form-tabs');
-
-        expect(tabs.children).toHaveLength(7);
-
-        const tabRow = queryByTitle('FormTabRow');
-
-        expect(tabRow.children).toHaveLength(1);
-    });
 });
diff --git a/packages/ra-ui-materialui/src/form/TabbedFormTabs.js b/packages/ra-ui-materialui/src/form/TabbedFormTabs.js
@@ -14,7 +14,6 @@ const TabbedFormTabs = ({
     classes,
     url,
     tabsWithErrors,
-    scrollable,
     ...rest
 }) => {
     const location = useLocation();
@@ -35,18 +34,8 @@ const TabbedFormTabs = ({
         ? location.pathname
         : validTabPaths[0];
 
-    const scrollableProps = scrollable
-        ? { className: classes.scrollableTabs }
-        : {};
-
     return (
-        <Tabs
-            aria-label="Form-tabs"
-            value={tabValue}
-            indicatorColor="primary"
-            {...scrollableProps}
-            {...rest}
-        >
+        <Tabs value={tabValue} indicatorColor="primary" {...rest}>
             {Children.map(children, (tab, index) => {
                 if (!isValidElement(tab)) return null;