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

fix: do not hide chaff when resizing #6916

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Conversation

maribethb
Copy link
Contributor

@maribethb maribethb commented Mar 22, 2023

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 #6915

See that issue for a lot of additional context.

Proposed Changes

  • Doesn't hide all the chaff during a resize. In particular, tries to keep the dropdown div and widget div open.
  • The dropdown div already had some unused logic to reposition itself. So I just called that logic. (For the history of this code, see the linked issue)
  • The widget div is trickier. We claim not to handle positioning for the widget div, unlike the dropdown div. So we shouldn't do repositioning logic directly in the widget div (which is what Scratch does, but it's not ideal). So I've added a new method that fields can override. By default, it returns false which means the widget div will hide itself, the same as today. But if you override this method and return true, that means the field will handle its own reposition logic.
  • Override the above mentioned method in field_input to resize themselves. This means subclasses of this field (text, multiline text, number, and angle, along with any custom developer fields) will resize the widget div automatically (by calling the existing resizeEditor_ method).
    • Also, the block owning these fields will bump themselves into the visible region of the workspace first. This prevents problems where the widget div is rendered outside of the workspace and onto the application's UI (this is the behavior the Scratch fix has and I wanted to avoid).
  • Renamed some parameters and variables inside a bump_objects method to make it more generic. Previously this method was only used to bump things into the scrollable region of the workspace. We need the exact same logic to bump it into the visible region of the workspace, and you can achieve this just by passing a different set of bounds into the function.

Behavior Before Change

On resizing a window while you have a widget or dropdown div open, they would be hidden. This causes the soft keyboard on Android to make fields unusable.

Behavior After Change

The dropdown and widget divs attempt to keep themselves open. This allows you to resize the window while editing an input field, or use fields on Android with the soft keyboard.

Reason for Changes

bug fixes and because some projects don't want to use the modal editor

Test Coverage

I tested this by resizing the window while the number, text, multiline text, and angle blocks were being edited, on desktop Chrome, mobile Safari, and mobile Chrome on Android. On both mobile browsers, I also tested rotating the device while these fields were being edited.

I also tested in the multi_playground to ensure this fix works with multiple workspaces present and in RTL mode. I also checked that the context menu (which uses the widget div) closes still when resizing.

Documentation

No, but the modalInputs option hasn't been documented yet. It is probably better that way since it wasn't really usable before this fix, but I'll add that to my todo list.

Additional Information

I'm investigating a UI bug that appears in text fields in RTL mode. I'm not sure why this code would cause it, so I need to check if it's happening on develop before I merge this. After investigation, this issue is happening on develop as well (though not on the published playgrounds on GitHub or App Engine) so I'll open a separate issue for that.

@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Apr 13, 2023
@maribethb maribethb marked this pull request as ready for review April 13, 2023 01:42
@maribethb maribethb requested a review from a team as a code owner April 13, 2023 01:42
@maribethb maribethb requested a review from cpcallen April 13, 2023 01:42
@maribethb
Copy link
Contributor Author

Including @rachel-fenichel on the review because she squinted at me when I said I was going to reuse the bumpIntoBounds function.

@maribethb maribethb merged commit 2bbb3aa into google:develop Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Soft keyboard issues on Android
3 participants