fix!: update the scroll options to work with v11 dragging

[BeksOmega]

The basics

• I validated my changes

The details

Resolves

Fixes #2317

Proposed Changes

Makes it so the scroll-options plugin works with the new dragging system.

Test Coverage

Tested manually with changes in core. Dragging workspace comments to the edge of the workspace properly scrolls yay!

Documentation

N/A

Additional Information

Dependent on google/blockly#8016, but we have no tests so we can still merge this.

Breaking changes / To Fix

The ScrollBlockDragger now conforms to the new IDraggable interfaces in v11 of core Blockly.

When constructing an AutoScroll you now have to pass the ScrollBlockDragger as well.

[maribethb]

LGTM, but can you update the PR description with a breaking change section? This is only a breaking change if you're subclassing elements of this plugin and if so then the class has been renamed. If you're simply installing and configuring this plugin, you don't need to take any action [edited].

[BeksOmega]

LGTM, but can you update the PR description with a breaking change section? This is only a breaking change if you're subclassing elements of this plugin and if so then the class has been renamed. If you're simply installing and configuring this plugin, you don't need to take any action [edited].

I think it's a breaking change either way because of the renaming. To pass it to the configuration structure you need to get it by name. Do you want me to not rename it?

[maribethb]

LGTM, but can you update the PR description with a breaking change section? This is only a breaking change if you're subclassing elements of this plugin and if so then the class has been renamed. If you're simply installing and configuring this plugin, you don't need to take any action [edited].

I think it's a breaking change either way because of the renaming. To pass it to the configuration structure you need to get it by name. Do you want me to not rename it?

Hmm, yeah. I think the value gained in renaming it is lower than the annoyance of making developers rename their plugin struct. The rename is more accurate but there's little value in that to the plugin user imo. I'd probably keep the name and have a comment explaining it now can drag more than blocks.

I think renames that affect people subclassing these classes or doing more advanced work are ok. But we should try not to break people who are just using the plugin as-is unless we have good reason. Open to debate on this point though :)

[BeksOmega]

LGTM, but can you update the PR description with a breaking change section? This is only a breaking change if you're subclassing elements of this plugin and if so then the class has been renamed. If you're simply installing and configuring this plugin, you don't need to take any action [edited].

I think it's a breaking change either way because of the renaming. To pass it to the configuration structure you need to get it by name. Do you want me to not rename it?

Hmm, yeah. I think the value gained in renaming it is lower than the annoyance of making developers rename their plugin struct. The rename is more accurate but there's little value in that to the plugin user imo. I'd probably keep the name and have a comment explaining it now can drag more than blocks.

I think renames that affect people subclassing these classes or doing more advanced work are ok. But we should try not to break people who are just using the plugin as-is unless we have good reason. Open to debate on this point though :)

Ehh as a team, we have this same argument every time we want to rename something. And then it's like "do we want to create an alias?" "well aliases are also annoying" etc etc. And it goes on forever. I'd rather just get this merged.

[maribethb]

Yes, but in the past we've also decided not to do renames that don't add a lot of value to the end-user. Most recently I recall were some changes in generators where we decided not to do renames and not to add aliases. So I don't think this decision is inconsistent.

[BeksOmega]

Yes, but in the past we've also decided not to do renames that don't add a lot of value to the end-user. Most recently I recall were some changes in generators where we decided not to do renames and not to add aliases. So I don't think this decision is inconsistent.

Oh to clarify I'm going to make the change you suggested! I was just saying I don't need to discuss it more and was just going to go with whatever you said haha.

[BeksOmega]

@maribethb Can you give this another quick look since I made the naming changes?

[maribethb]

I think the 3 places I noted still need to be updated then lgtm

[maribethb]

this name needs to be updated

[BeksOmega]

oh hm I guess rename symbol doesn't work for jsdoc tags?