2
\$\begingroup\$

In my react application I have Tabs done with Material UI. Some of those tabs are shown or hide depending on an option is true or false. The code is not a clean solution but I have no experience working with this and was wondering to see a better solution to what I have done.

We have 3 options:

  • isEngagementPortal
  • isRecruitmentPortal
  • isConsenteesPortal

when one of the above options is true/false the corresponding tab is shown or hide.

I was thinking to make it in smaller components or dynamic but have difficulties doing it.

<div className={classes.root}>
      <AppBar className={classes.appBar} position="static" elevation={0}>
        <Toolbar disableGutters>
          <Tabs value={tab} onChange={(e, newValue) => setTab(newValue)}>
            <Tab label="Study Info" />
            <Tab label="Study Tracks" />
            <Tab label="User Admin" />
            <Tab label="Study Locales" />
            {isEngagementPortal && <Tab label="Resources" />}
            {isRecruitmentPortal && <Tab label="Pre-screener" />}
            {isRecruitmentPortal && <Tab label="Consent" />}
            {isRecruitmentPortal && <Tab label="Manuscript" />}
            <Tab label="Survey" />
            {isRecruitmentPortal && <Tab label="Translations" />}
            <Tab label="Sites" />
            {isConsenteesPortal && <Tab label="eConsent" />}
            {isRecruitmentPortal && <Tab label="Reports" />}
          </Tabs>
        </Toolbar>
      </AppBar>
      {console.log(tab)}
      <Grid>
        {tab === 0 && (
          <StudyInfo
            studyId={studyId}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
          />
        )}
        {tab === 1 && <StudyTracks studyId={studyId} />}
        {tab === 2 && <div>USER ADMIN</div>}
        {tab === 3 && (
          <StudyLocales
            setAlert={setAlert}
            studyId={studyId}
            locales={locales}
            currentLocales={currentLocales}
            refetchQueries={refetchQueries}
          />
        )}
        {isEngagementPortal && tab === 4 && (
          <StudyResources studyId={studyId} locales={locales} />
        )}
        {tab === 5 && isRecruitmentPortal && (
          <Questionnaire
            type="PRESCREENER"
            preview
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 6 && (
          <Questionnaire
            type="CONSENT"
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 7 && (
          <Questionnaire
            type="MANUSCRIPT"
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {tab === 8 && (
          <Questionnaire
            type="SURVEY"
            preview
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 9 && (
          <StudyTranslation
            studyId={studyId}
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
          />
        )}
        {tab === 10 && (
          <Sites
            studyId={studyId}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            locales={locales}
          />
        )}
        {isConsenteesPortal && tab === 11 && <div>E-CONSENT</div>}
        {isRecruitmentPortal && tab === 12 && <Reports studyId={studyId} />}
      </Grid>
      <Alert content={alert} closeDialog={setAlert} />
    </div>
  );  

As requested the full component

/* eslint-disable react/prop-types */
import { useState } from 'react';
import {
  AppBar,
  Grid,
  Tab,
  Tabs,
  Toolbar,
  makeStyles,
} from '@material-ui/core';
import { get, map } from 'lodash';
import { useQuery } from '@apollo/react-hooks';
import Alert from '../../Dialogs/Alert';
import Loading from '../../Loading';
import StudyInfo from './StudyTabs/StudyInfo';
import StudyTranslation from './StudyTabs/StudyTranslations';
import Questionnaire from './StudyTabs/Questionnaire';
import Sites from './StudyTabs/Sites';
import studyRefetchQueries from './StudyTabs/utils/refetchQueries';
import StudyLocales from './StudyTabs/StudyLocales';
import StudyTracks from './StudyTabs/StudyTracks';
import { getLocales, getFeatureOptions } from './StudyTabs/utils/queries';
import Reports from './StudyTabs/Reports';
import StudyResources from './StudyTabs/StudyResources';

const useStyles = makeStyles(theme => ({
  root: {
    flexGrow: 1,
    backgroundColor: theme.palette.background.paper,
    color: 'white',
  },
  appBar: {
    backgroundColor: theme.palette.background.paper,
  },
}));

function Study({ studyId }) {
  console.log(studyId);
  const classes = useStyles();
  const { loading, data } = useQuery(getLocales, { variables: { studyId } });
  const { data: featureOptionsData } = useQuery(getFeatureOptions, {
    variables: { studyId },
  });
  const [alert, setAlert] = useState();
  const [tab, setTab] = useState(0);
  const [showTab, setShowTab] = useState(false);

  if (loading) {
    return <Loading />;
  }

  const { refetchQueries } = studyRefetchQueries({ studyId });
  const locales = get(data, 'locales.nodes') || [];
  const currentLocales = map(locales, l => l.locale);

  const options = get(featureOptionsData, 'featureOptions') || [];

  console.log(options);

  const isRecruitmentPortal = options.recruitmentPortal;
  const isEngagementPortal = options.engagementPortal;
  const isConsenteesPortal = options.consenteesPortal;

  console.log(isRecruitmentPortal, isEngagementPortal, isConsenteesPortal);

  return (
    <div className={classes.root}>
      <AppBar className={classes.appBar} position="static" elevation={0}>
        <Toolbar disableGutters>
          <Tabs value={tab} onChange={(e, newValue) => setTab(newValue)}>
            <Tab value={0} label="Study Info" />
            <Tab value={1} label="Study Tracks" />
            <Tab value={2} label="User Admin" />
            <Tab value={3} label="Study Locales" />
            {isEngagementPortal && <Tab value={4} label="Resources" />}
            {isRecruitmentPortal && <Tab value={5} label="Pre-screener" />}
            {isRecruitmentPortal && <Tab value={6} label="Consent" />}
            {isRecruitmentPortal && <Tab value={7} label="Manuscript" />}
            <Tab value={8} label="Survey" />
            {isRecruitmentPortal && <Tab value={9} label="Translations" />}
            <Tab value={10} label="Sites" />
            {isConsenteesPortal && <Tab value={11} label="eConsent" />}
            {isRecruitmentPortal && <Tab value={12} label="Reports" />}
          </Tabs>
        </Toolbar>
      </AppBar>
      {console.log(tab)}
      <Grid>
        {tab === 0 && (
          <StudyInfo
            studyId={studyId}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
          />
        )}
        {tab === 1 && <StudyTracks studyId={studyId} />}
        {tab === 2 && <div>USER ADMIN</div>}
        {tab === 3 && (
          <StudyLocales
            setAlert={setAlert}
            studyId={studyId}
            locales={locales}
            currentLocales={currentLocales}
            refetchQueries={refetchQueries}
          />
        )}
        {isEngagementPortal && tab === 4 && (
          <StudyResources studyId={studyId} locales={locales} />
        )}
        {tab === 5 && isRecruitmentPortal && (
          <Questionnaire
            type="PRESCREENER"
            preview
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 6 && (
          <Questionnaire
            type="CONSENT"
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 7 && (
          <Questionnaire
            type="MANUSCRIPT"
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {tab === 8 && (
          <Questionnaire
            type="SURVEY"
            preview
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            studyId={studyId}
          />
        )}
        {isRecruitmentPortal && tab === 9 && (
          <StudyTranslation
            studyId={studyId}
            locales={currentLocales}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
          />
        )}
        {tab === 10 && (
          <Sites
            studyId={studyId}
            refetchQueries={refetchQueries}
            setAlert={setAlert}
            locales={locales}
          />
        )}
        {isConsenteesPortal && tab === 11 && <div>E-CONSENT</div>}
        {isRecruitmentPortal && tab === 12 && <Reports studyId={studyId} />}
      </Grid>
      <Alert content={alert} closeDialog={setAlert} />
    </div>
  );
}

export default Study;
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Could you post the entire component? It looks like you're missing part of it. \$\endgroup\$ – Scotty Jamison Feb 2 at 23:32
  • 1
    \$\begingroup\$ @ScottyJamison Added the full component please have a look :) \$\endgroup\$ – Jakub Feb 3 at 8:03
2
\$\begingroup\$

Each tab seems to have three pieces of data/logic associated with it:

  • The tab label
  • The tab content
  • When to render it

A good way to simplify your component would be to actually group these bits of related logic together. This can be done by using an array of objects, as an example.

Some other suggestions to help improve your overall component:

  • Prefer using standard javascript features over lodash ones when convenient. i.e. just use array.map(fn) instead of lodash's map(array, fn). More people know how to use the standard language features than lodash-specific functions, so it makes the code easier to read and maintain by other people.
  • Likewise, if your target browsers support it, or if you're transpiling these features, you can use the optional chaining operator (?.) instead of lodash's get(). The nullish coalescing operator (??) can be used instead of || in some cases too.
  • At one point you're getting the featureOptions property from an object, and defaulting to an empty array. You then access different values from the result. Did you mean to default to an empty object?
  • Things like network resources can be taken out into it's own custom hook to try and move out some of the slightly lower-level logic from the main component.

The following is an example that puts these suggestions together.

const Study = ({ studyId }) => {
  const classes = useStyles();
  const { loading, options, locales } = useNetworkResources(studyId)
  const [alert, setAlert] = useState();
  const [selectedTabId, setSelectedTabId] = useState(0);

  const visibleTabs = tabs.filter(tab => tab.isVisible(options));
  const selectedTab = visibleTabs[selectedTabId]

  if (loading) {
    return <Loading />;
  }
  
  return (
    <div className={classes.root}>
      <TabBar
        selectedTabId={selectedTabId}
        setSelectedTabId={setSelectedTabId}
        visibleTabs={visibleTabs}
      />
      <Grid>
        {selectedTab.isVisible(options) && (
          <selectedTab.Content
            setAlert={setAlert}
            studyId={studyId}
            locales={locales}
            currentLocales={locales.map(l => l.locale)}
            refetchQueries={studyRefetchQueries({ studyId }).refetchQueries}
          />
        )}
      </Grid>
      <Alert content={alert} closeDialog={setAlert} />
    </div>
  );
};

const TabBar = ({ selectedTabId, setSelectedTabId, visibleTabs }) => (
  <AppBar className={classes.appBar} position="static" elevation={0}>
    <Toolbar disableGutters>
      <Tabs value={selectedTabId} onChange={(e, newValue) => setSelectedTabId(newValue)}>
        {visibleTabs.map(({ label }) => <Tab key={label} label={label} />)}
      </Tabs>
    </Toolbar>
  </AppBar>
);

const useNetworkResources = studyId => {
  const { loading, data: rawLocalesData } = useQuery(getLocales, { variables: { studyId } });
  const { data: rawOptionsData } = useQuery(getFeatureOptions, { variables: { studyId }, });
  return {
    loading,
    options: rawOptionsData?.featureOptions ?? {},
    locales: rawLocalesData?.locales?.nodes ?? [],
  };
};

const tabs = [
  {
    label: 'Study Info',
    isVisible: () => true,
    Content: ({ studyId, refetchQueries, setAlert }) =>(
      <StudyInfo
        studyId={studyId}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
      />
    ),
  },
  {
    label: 'Study Tracks',
    isVisible: () => true,
    Content: ({ studyId }) => <StudyTracks studyId={studyId} />,
  },
  {
    label: 'User Admin',
    isVisible: () => true,
    Content: () => <div>USER ADMIN</div>,
  },
  {
    label: 'Study Locales',
    isVisible: () => true,
    Content: ({ setAlert, studyId, locales, currentLocales, refetchQueries }) => (
      <StudyLocales
        setAlert={setAlert}
        studyId={studyId}
        locales={locales}
        currentLocales={currentLocales}
        refetchQueries={refetchQueries}
      />
    ),
  },
  {
    label: 'Resources',
    isVisible: options => options.isEngagementPortal,
    Content: ({ studyId, locales }) => <StudyResources studyId={studyId} locales={locales} />,
  },
  {
    label: 'Pre-screener',
    isVisible: options => options.isRecruitmentPortal,
    Content: ({ currentLocales, refetchQueries, setAlert, studyId }) => (
      <Questionnaire
        type="PRESCREENER"
        preview
        locales={currentLocales}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
        studyId={studyId}
      />
    ),
  },
  {
    label: 'Consent',
    isVisible: options => options.isRecruitmentPortal,
    Content: ({ currentLocales, refetchQueries, setAlert, studyId }) => (
      <Questionnaire
        type="CONSENT"
        locales={currentLocales}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
        studyId={studyId}
      />
    ),
  },
  {
    label: 'Manuscript',
    isVisible: options => options.isRecruitmentPortal,
    Content: ({ currentLocales, refetchQueries, setAlert, studyId }) => (
      <Questionnaire
        type="MANUSCRIPT"
        locales={currentLocales}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
        studyId={studyId}
      />
    ),
  },
  {
    label: 'Survey',
    isVisible: () => true,
    Content: ({ currentLocales, refetchQueries, setAlert, studyId }) => (
      <Questionnaire
        type="SURVEY"
        preview
        locales={currentLocales}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
        studyId={studyId}
      />
    ),
  },
  {
    label: 'Translations',
    isVisible: options => options.isRecruitmentPortal,
    Content: ({ studyId, currentLocales, refetchQueries, setAlert }) => (
      <StudyTranslation
        studyId={studyId}
        locales={currentLocales}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
      />
    ),
  },
  {
    label: 'Sites',
    isVisible: () => true,
    Content: () => (
      <Sites
        studyId={studyId}
        refetchQueries={refetchQueries}
        setAlert={setAlert}
        locales={locales}
      />
    ),
  },
  {
    label: 'eConsent',
    isVisible: options => options.isConsenteesPortal,
    Content: () => <div>E-CONSENT</div>,
  },
  {
    label: 'Reports',
    isVisible: options => options.isRecruitmentPortal,
    Content: ({ studyId }) => <Reports studyId={studyId} />,
  },
];

Update Took out logic and explanation having to do with a faulty assumption of how the O.P.'s three booleans were used. (The O.P. corrected me in the comments). Also added more feedback because the O.P. provided the entire component to review, and fixed some bugs (also mentioned in the comments).

\$\endgroup\$
9
  • \$\begingroup\$ Seems nice what you add I need to test. The assumption is that we can have all of them true as false. Or like one true and the rest false or so on. All depends on the setup configuration as a user can decide to have all, one or some of the portals activated, I added my full component to have a look :) \$\endgroup\$ – Jakub Feb 3 at 8:08
  • \$\begingroup\$ I tested but I'm stuck on this const selectedTab = tabs[selectedTab]; it is saying is undefined and that was used before is defined and don't know what to do. Here you can see my changes pastebin.com/E5P5tGR8 \$\endgroup\$ – Jakub Feb 3 at 9:41
  • \$\begingroup\$ Sorry, that was a mess-up, I didn't test the code above. Do const selectedTab = tabs[selectedTabId] \$\endgroup\$ – Scotty Jamison Feb 3 at 10:05
  • \$\begingroup\$ Thanks but now I have another issue. When I hide the tabs the corresponding content is not aligned with the tab. For example I hide the Pre-screener tab and what is about that. When I click on the Survey tab I see that pre-screenr as selectedTab and not Survey. \$\endgroup\$ – Jakub Feb 3 at 10:15
  • 1
    \$\begingroup\$ Yes thanks I saw the comments and regarding lodash parts I'm doing to be consistent on how the project is done before me. The featureOptions are coming from a GraphQL query and as default are all false so in reality I think will never be an empty array. The network hook thing is a good idea but I need to discuss internally if can move like that :) but was great explanations. \$\endgroup\$ – Jakub Feb 3 at 10:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.