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: scroll-options fixes and compatibility with custom draggables #2358

Merged
merged 8 commits into from
May 16, 2024

Conversation

maribethb
Copy link
Contributor

@maribethb maribethb commented May 16, 2024

The basics

The details

Resolves

Fixes #2354

Proposed Changes

  • Converts the plugin to TypeScript because type guards are a good solution to these problems
  • Adds type guards for the metrics manager and custom draggables
  • Updates the reference to Dragger base class to account for the new location in core
  • Adds workspace comments to test page for easier testing
  • Adds info to readme about how to use this with other metrics managers and with custom draggables

Reason for Changes

bug bash

no more errors when trying to drag bubbles!

Test Coverage

manual testing

Documentation

updated readme

Additional Information

requesting review from beka to make sure my warning about the custom draggable objects seems legit

also, we had discussed offline if using drag instead of moveDuringDrag would be sufficient, since drag is already present on hte IDraggable interface, and moveDuringDrag is internal. turns out it works fine!

@maribethb maribethb requested a review from BeksOmega May 16, 2024 00:02
@maribethb maribethb requested a review from a team as a code owner May 16, 2024 00:02
@BeksOmega
Copy link
Contributor

Discussed offline: this needs some early returns so it doesn't throw errors for things that don't have all of the methods it needs. This is because bubbles are draggable and don't have all of the methods it needs :P

Didn't get caught before because this is in JS so we don't get type errors, and I only tested with blocks and workspace comments :/

@maribethb maribethb changed the title fix: fix scroll-options base class fix: scroll-options fixes and compatibility with custom draggables May 16, 2024
@maribethb
Copy link
Contributor Author

Ready for review!

@maribethb maribethb merged commit 2da6080 into google:rc/v11.0.0 May 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants