Skip to content

Conversation

@RoboErikG
Copy link
Contributor

This makes it easier to scroll something into view with context by including the padding before checking if it should be scrolled into view.

The basics

The details

Resolves

Fixes an issue uncovered in RaspberryPiFoundation/blockly-keyboard-experimentation#451

Proposed Changes

Add the padding to the bounds before checking if it's within the viewport.

Reason for Changes

During a keyboard drag this method may be called more once. When that happens only the first call with default padding is executed. During a constrained drag we'd like to increase the padding to give more context about the new location, so we should check with the padding to make sure later calls that try to move it further in bounds are respected.

Test Coverage

Documentation

Additional Information

This makes it easier to scroll something into view with context by
including the padding before checking if it should be scrolled into
view.
@microbit-matt-hillsdon
Copy link
Collaborator

I am not sure this is always desirable. Blocks get scrolled into view by keyboard nav on selection, which can just be defaulted e.g. when you move to the workspace. There are often blocks near the origin. So this can mean the workspace jumps by padding unexpectedly.

Pretty sure it used to be this way and I changed it because it felt glitchy.

Whether you want to include the padding might depend on the action you're performing. In the context of a move it's much more reasonable.

bounds.bottom <= viewport.bottom
) {
// Do nothing if the block is fully inside the viewport.
// Do nothing if the block with padding is fully inside the viewport.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit while this comment is being edited: should be "bounds", not "block"

@RoboErikG
Copy link
Contributor Author

Good point @microbit-matt-hillsdon . An alternate solution would be to add it to the bounds before calling this method to force the move and leaving this method as is. I'll play around with it after @microbit-grace gets her change in.

@RoboErikG
Copy link
Contributor Author

Resolved this via RaspberryPiFoundation/blockly-keyboard-experimentation#474 instead

@RoboErikG RoboErikG closed this Apr 29, 2025
@RoboErikG RoboErikG deleted the bounds-with-padding branch June 23, 2025 16:55
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.

4 participants