Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: add option to disable modal inputs on mobile #6625

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

maribethb
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes #6608

Proposed Changes

Adds an option called modalInputs to BlocklyOptions (and Options). Default is true. If false, then FieldTextInput and its subclasses will not show the modal prompt editor when on mobile browsers.

Behavior Before Change

Always show the modal editor if on mobile browser.

Behavior After Change

Default is to show modal editor if on mobile, but this can be disabled by setting the new option to false. If it's false, we'll show the inline editor like we would on desktop.

Reason for Changes

Not everyone wants the modal UI, especially these days when the mobile keyboard experience is not as bad as it used to be (e.g. smart bumping of content now)

Alternatives Considered

  • Not adding a new BlocklyOption to the giant config struct... but ultimately this is a piece of configuration so it needs to be configured somewhere, and the least surprising place would be the options struct.
  • Making people subclass FieldTextInput to not show the prompt editor. This would be fine, except you also then have to subclass all of its children as well, and add the exact same logic to each of those classes, which is pretty messy for something that should be a single boolean check.
  • Somehow taking advantage of the logic for setting a custom prompt, and letting people set a custom prompt that would return false or something and then we'd know to show the inline editor instead. This would require more changes than the chosen solution and is quite convoluted logic, plus has the ability to interfere with other custom prompts a user may want to keep.

Test Coverage

Tested manually in the playground. I looked at adding this to the advanced playground UI in core but as far as I can tell you can't modify the existing folders and I didn't want to make another new one. After this is released, it could be added as an option in the advanced playground in samples.

Documentation

Needs to be documented in the devsite article about the configuration struct.

Additional Information

Please fight over the name of the option in the comments.

@maribethb maribethb requested a review from a team as a code owner November 15, 2022 00:03
@maribethb maribethb requested a review from BeksOmega November 15, 2022 00:03
@github-actions github-actions bot added the PR: feature Adds a feature label Nov 15, 2022
Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Please fight over the name of the option in the comments.

useModalInputs? /s 😛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to not use modal input on mobile
2 participants