Repository: freeCodeCamp/freeCodeCamp
File: client/src/components/Donation/donation-modal.tsx (TypeScript)

Code before:
  executeGA,
  location,
  recentlyClaimedBlock
}: DonateModalProps): JSX.Element {
  const [closeLabel, setCloseLabel] = useState(false);
  const [ctaNumber, setCtaNumber] = useState(0);
  const [isDisabled, setIsDisabled] = useState(true);
  const [showSkipButton, setShowSkipButton] = useState(false);
  const loadElementsIdividually = useFeature('load_elements_individually').on;
  const { t } = useTranslation();
  const handleProcessing = () => {
    setCloseLabel(true);
  };

  useEffect(() => {
    if (!loadElementsIdividually) {
      setIsDisabled(false);
      setShowSkipButton(true);
      return;
    }
    const timer = setTimeout(() => {
      setIsDisabled(false);
      setShowSkipButton(true);
    }, 4000);
    return () => clearTimeout(timer);
  }, [loadElementsIdividually]);

  useEffect(() => {
    if (show) {
      void playTone('donation');
      executeGA({ event: 'pageview', pagePath: '/donation-modal' });
      executeGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock, executeGA]);

  useEffect(() => {
    if (show) setCtaNumber(getctaNumberBetween1To10());
  }, [show]);

  const handleModalHide = () => {
    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {
      goToAnchor('claim-cert-block');
    }
  };


Review comment (suggestion):
```suggestion
    if (loadElementsIndividually) {
      const timer = setTimeout(() => {
        setIsDisabled(false);
        setShowSkipButton(true);
      }, 4000);
      return () => clearTimeout(timer);
    } else {
      setIsDisabled(false);
      setShowSkipButton(true);
    }
  }, [loadElementsIndividually]);
```
Generally I think it's nicer to avoid negation if possible (less thinking required). Also, I like early returns, but I find they get less and less clear the further the `return` gets from the condition.

Code after:

function DonateModal({
  show,
  closeDonationModal,
  executeGA,
  location,
  recentlyClaimedBlock
}: DonateModalProps): JSX.Element {
  const [closeLabel, setCloseLabel] = useState(false);
  const [ctaNumber, setCtaNumber] = useState(0);
  const [isDisabled, setIsDisabled] = useState(true);
  const [showSkipButton, setShowSkipButton] = useState(false);
  const loadElementsIndividually = useFeature('load_elements_individually').on;
  const { t } = useTranslation();
  const handleProcessing = () => {
    setCloseLabel(true);
  };

  useEffect(() => {
    if (loadElementsIndividually) {
      const timer = setTimeout(() => {
        setIsDisabled(false);
        setShowSkipButton(true);
      }, 4000);
      return () => clearTimeout(timer);
    } else {
      setIsDisabled(false);
      setShowSkipButton(true);
    }
  }, [loadElementsIndividually]);

  useEffect(() => {
    if (show) {
      void playTone('donation');
      executeGA({ event: 'pageview', pagePath: '/donation-modal' });
      executeGA({
        event: 'donation_view',
        action: `Displayed ${
          recentlyClaimedBlock !== null ? 'Block' : 'Progress'
        } Donation Modal`
      });
    }
  }, [show, recentlyClaimedBlock, executeGA]);

  useEffect(() => {
    if (show) setCtaNumber(getctaNumberBetween1To10());
  }, [show]);

  const handleModalHide = () => {
    // If modal is open on a SuperBlock page
    if (isLocationSuperBlock(location)) {