Skip to content

Create OnOffToggle component for preferences. #2323

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

lindapaiste
Copy link
Collaborator

@lindapaiste lindapaiste commented Jul 24, 2023

This is purely a code cleanup, there is no change to UI or behavior. It extracts a lot of repetitive code in the Preferences component into a new component OnOffToggle.

The component is inspired by @ghalestrilo's optionsOnOff code for the mobile preferences (which will be going away, in favor of a single responsive component).

It will probably evolve more as I refactor other parts of the Preferences.

Changes:

  • Create a component OnOffToggle for the clickable "On" and "Off" labels.
    image
  • Write unit tests.
  • Use the OnOffToggle in Preferences for Autosave, Autoclose Brackets and Quotes, etc.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Collaborator Author

lindapaiste commented Jul 24, 2023

Maybe I should rework the PreferencePicker instead of making a whole new component? 🤷

Copy link
Contributor

@PiyushChandra17 PiyushChandra17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely Done

…noff

# Conflicts:
#	client/modules/IDE/components/Preferences/index.jsx
@lindapaiste
Copy link
Collaborator Author

Possibly translationKey="LineNumbers" is a bit too "tricky" and it would be be better to be more explicit, even though it's more verbose. I can change the syntax to ariaLabelOn={t('Preferences.LineNumbersOnARIA')} ariaLabelOff={t('Preferences.LineNumbersOffARIA')}.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants