From ef78b9bf178bd279ec4def0e1f330e09f92df630 Mon Sep 17 00:00:00 2001 From: Linda Paiste Date: Mon, 24 Jul 2023 17:30:00 -0500 Subject: [PATCH 1/2] Create OnOffToggle component for preferences. --- .../components/Preferences/OnOffToggle.jsx | 74 +++++ .../Preferences/OnOffToggle.test.jsx | 65 +++++ .../IDE/components/Preferences/index.jsx | 256 +++--------------- 3 files changed, 177 insertions(+), 218 deletions(-) create mode 100644 client/modules/IDE/components/Preferences/OnOffToggle.jsx create mode 100644 client/modules/IDE/components/Preferences/OnOffToggle.test.jsx diff --git a/client/modules/IDE/components/Preferences/OnOffToggle.jsx b/client/modules/IDE/components/Preferences/OnOffToggle.jsx new file mode 100644 index 0000000000..ae220da139 --- /dev/null +++ b/client/modules/IDE/components/Preferences/OnOffToggle.jsx @@ -0,0 +1,74 @@ +import PropTypes from 'prop-types'; +import React from 'react'; +import { useTranslation } from 'react-i18next'; + +const OnOffToggle = ({ value, setValue, name, translationKey, children }) => { + const { t } = useTranslation(); + + return ( +
+ setValue(true)} + aria-label={t(`Preferences.${translationKey}OnARIA`)} + name={name} + id={`${name}-on`} + className="preference__radio-button" + value="On" + checked={value} + /> + + setValue(false)} + aria-label={t(`Preferences.${translationKey}OffARIA`)} + name={name} + id={`${name}-off`} + className="preference__radio-button" + value="Off" + checked={!value} + /> + + {children} +
+ ); +}; + +OnOffToggle.propTypes = { + /** + * `true` if turned on, `false` if off. + */ + value: PropTypes.bool.isRequired, + /** + * Function to call when the value is changed. + * Will receive the new `boolean` value. + */ + setValue: PropTypes.func.isRequired, + /** + * Used as the HTML `name` attribute of the form elements, + * and also used to formulate the `id`s. + */ + name: PropTypes.string.isRequired, + /** + * Common prefix for looking up the "On" and "Off" ARIA labels. + * If the ARIA label is t('Preferences.LintWarningOnARIA'), + * then the `translationKey` is `LintWarning`. + */ + translationKey: PropTypes.string, + /** + * Can insert additional elements in the same
as the inputs. + * Typically not used. + */ + children: PropTypes.node +}; + +OnOffToggle.defaultProps = { + translationKey: '', + children: null +}; + +export default OnOffToggle; diff --git a/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx b/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx new file mode 100644 index 0000000000..4a035934e4 --- /dev/null +++ b/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx @@ -0,0 +1,65 @@ +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { render, screen, fireEvent } from '../../../../test-utils'; +import OnOffToggle from './OnOffToggle'; + +describe('OnOffToggle', () => { + const setValue = jest.fn(); + + const subject = (initialValue = false) => { + const result = render( + + ); + return { + ...result, + onButton: screen.getByLabelText('On'), + offButton: screen.getByLabelText('Off') + }; + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('highlights "On" when value is true', () => { + const { onButton, offButton } = subject(true); + + expect(onButton).toBeChecked(); + expect(offButton).not.toBeChecked(); + }); + + it('highlights "Off" when value is false', () => { + const { onButton, offButton } = subject(false); + + expect(onButton).not.toBeChecked(); + expect(offButton).toBeChecked(); + }); + + it('does nothing when clicking the active value', () => { + const { onButton } = subject(true); + + act(() => { + fireEvent.click(onButton); + }); + + expect(setValue).not.toHaveBeenCalled(); + }); + + it('calls setValue when clicking the inactive value', () => { + const { offButton } = subject(true); + + act(() => { + fireEvent.click(offButton); + }); + + expect(setValue).toHaveBeenCalledTimes(1); + expect(setValue).toHaveBeenCalledWith(false); + + // Note: the checked state will not change until the `value` prop changes. + }); +}); diff --git a/client/modules/IDE/components/Preferences/index.jsx b/client/modules/IDE/components/Preferences/index.jsx index 87d0a28b9d..2c623a87a7 100644 --- a/client/modules/IDE/components/Preferences/index.jsx +++ b/client/modules/IDE/components/Preferences/index.jsx @@ -10,14 +10,11 @@ import { withTranslation } from 'react-i18next'; import PlusIcon from '../../../../images/plus.svg'; import MinusIcon from '../../../../images/minus.svg'; import beepUrl from '../../../../sounds/audioAlert.mp3'; +import OnOffToggle from './OnOffToggle'; class Preferences extends React.Component { constructor(props) { super(props); - this.handleUpdateAutosave = this.handleUpdateAutosave.bind(this); - this.handleUpdateLinewrap = this.handleUpdateLinewrap.bind(this); - this.handleLintWarning = this.handleLintWarning.bind(this); - this.handleLineNumbers = this.handleLineNumbers.bind(this); this.onFontInputChange = this.onFontInputChange.bind(this); this.onFontInputSubmit = this.onFontInputSubmit.bind(this); this.increaseFontSize = this.increaseFontSize.bind(this); @@ -68,26 +65,6 @@ class Preferences extends React.Component { this.setFontSize(newValue); } - handleUpdateAutosave(event) { - const value = event.target.value === 'true'; - this.props.setAutosave(value); - } - - handleUpdateLinewrap(event) { - const value = event.target.value === 'true'; - this.props.setLinewrap(value); - } - - handleLintWarning(event) { - const value = event.target.value === 'true'; - this.props.setLintWarning(value); - } - - handleLineNumbers(event) { - const value = event.target.value === 'true'; - this.props.setLineNumbers(value); - } - render() { const beep = new Audio(beepUrl); @@ -217,153 +194,45 @@ class Preferences extends React.Component {

{this.props.t('Preferences.Autosave')}

-
- this.props.setAutosave(true)} - aria-label={this.props.t('Preferences.AutosaveOnARIA')} - name="autosave" - id="autosave-on" - className="preference__radio-button" - value="On" - checked={this.props.autosave} - /> - - this.props.setAutosave(false)} - aria-label={this.props.t('Preferences.AutosaveOffARIA')} - name="autosave" - id="autosave-off" - className="preference__radio-button" - value="Off" - checked={!this.props.autosave} - /> - -
+

{this.props.t('Preferences.AutocloseBracketsQuotes')}

-
- this.props.setAutocloseBracketsQuotes(true)} - aria-label={this.props.t( - 'Preferences.AutocloseBracketsQuotesOnARIA' - )} - name="autoclosebracketsquotes" - id="autoclosebracketsquotes-on" - className="preference__radio-button" - value="On" - checked={this.props.autocloseBracketsQuotes} - /> - - this.props.setAutocloseBracketsQuotes(false)} - aria-label={this.props.t( - 'Preferences.AutocloseBracketsQuotesOffARIA' - )} - name="autoclosebracketsquotes" - id="autoclosebracketsquotes-off" - className="preference__radio-button" - value="Off" - checked={!this.props.autocloseBracketsQuotes} - /> - -
+

{this.props.t('Preferences.AutocompleteHinter')}

-
- this.props.setAutocompleteHinter(true)} - aria-label={this.props.t( - 'Preferences.AutocompleteHinterOnARIA' - )} - name="autocompletehinter" - id="autocompletehinter-on" - className="preference__radio-button" - value="On" - checked={this.props.autocompleteHinter} - /> - - this.props.setAutocompleteHinter(false)} - aria-label={this.props.t( - 'Preferences.AutocompleteHinterOffARIA' - )} - name="autocompletehinter" - id="autocompletehinter-off" - className="preference__radio-button" - value="Off" - checked={!this.props.autocompleteHinter} - /> - -
+

{this.props.t('Preferences.WordWrap')}

-
- this.props.setLinewrap(true)} - aria-label={this.props.t('Preferences.LineWrapOnARIA')} - name="linewrap" - id="linewrap-on" - className="preference__radio-button" - value="On" - checked={this.props.linewrap} - /> - - this.props.setLinewrap(false)} - aria-label={this.props.t('Preferences.LineWrapOffARIA')} - name="linewrap" - id="linewrap-off" - className="preference__radio-button" - value="Off" - checked={!this.props.linewrap} - /> - -
+
@@ -371,72 +240,23 @@ class Preferences extends React.Component {

{this.props.t('Preferences.LineNumbers')}

-
- this.props.setLineNumbers(true)} - aria-label={this.props.t('Preferences.LineNumbersOnARIA')} - name="line numbers" - id="line-numbers-on" - className="preference__radio-button" - value="On" - checked={this.props.lineNumbers} - /> - - this.props.setLineNumbers(false)} - aria-label={this.props.t('Preferences.LineNumbersOffARIA')} - name="line numbers" - id="line-numbers-off" - className="preference__radio-button" - value="Off" - checked={!this.props.lineNumbers} - /> - -
+

{this.props.t('Preferences.LintWarningSound')}

-
- this.props.setLintWarning(true)} - aria-label={this.props.t('Preferences.LintWarningOnARIA')} - name="lint warning" - id="lint-warning-on" - className="preference__radio-button" - value="On" - checked={this.props.lintWarning} - /> - - this.props.setLintWarning(false)} - aria-label={this.props.t('Preferences.LintWarningOffARIA')} - name="lint warning" - id="lint-warning-off" - className="preference__radio-button" - value="Off" - checked={!this.props.lintWarning} - /> - + -
+

From 23d147899d43d682ddccd65ddd1404b927d0e264 Mon Sep 17 00:00:00 2001 From: Linda Paiste Date: Mon, 7 Aug 2023 12:47:17 -0500 Subject: [PATCH 2/2] Remove `act` from tests. --- .../IDE/components/Preferences/OnOffToggle.test.jsx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx b/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx index 4a035934e4..05f76bb0b7 100644 --- a/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx +++ b/client/modules/IDE/components/Preferences/OnOffToggle.test.jsx @@ -1,5 +1,4 @@ import React from 'react'; -import { act } from 'react-dom/test-utils'; import { render, screen, fireEvent } from '../../../../test-utils'; import OnOffToggle from './OnOffToggle'; @@ -43,9 +42,7 @@ describe('OnOffToggle', () => { it('does nothing when clicking the active value', () => { const { onButton } = subject(true); - act(() => { - fireEvent.click(onButton); - }); + fireEvent.click(onButton); expect(setValue).not.toHaveBeenCalled(); }); @@ -53,9 +50,7 @@ describe('OnOffToggle', () => { it('calls setValue when clicking the inactive value', () => { const { offButton } = subject(true); - act(() => { - fireEvent.click(offButton); - }); + fireEvent.click(offButton); expect(setValue).toHaveBeenCalledTimes(1); expect(setValue).toHaveBeenCalledWith(false);