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 interfaces for focus management #8774

Merged

Conversation

BenHenning
Copy link
Contributor

The basics

The details

Resolves

Fixes #8772
Fixes #8773
Fixes part of #8770

Proposed Changes

Introduces the necessary base interfaces for representing different focusable contexts within Blockly. The actual logic for utilizing and implementing these interfaces will come in later PRs.

Reason for Changes

These interfaces provide the foundation for introducing a central system for managing focus (see #8770 for more context).

Test Coverage

N/A -- This is an interface-only change.

Documentation

Probably not--the class documentation should be sufficient.

Additional Information

See also google/blockly-keyboard-experimentation#142 (comment) for broader technical design context on the introduction of these new interfaces.

@BenHenning BenHenning requested a review from a team as a code owner February 14, 2025 01:34
@BenHenning BenHenning requested a review from gonfunko February 14, 2025 01:34
@github-actions github-actions bot added the PR: feature Adds a feature label Feb 14, 2025
@BenHenning
Copy link
Contributor Author

PTAL @gonfunko.

Also, will the circular import dependency here actually work? I didn't see any errors during build or in the playground, but since the new interfaces aren't actually being used (only exported) I wasn't sure if such errors would actually come up (I'd expect them to be caught during JavaScript generation from the TypeScript code, but I suppose there might be reasons it's ignored).

@gonfunko
Copy link
Contributor

LGTM, the import should be fine since it's just importing the type AFAIK.

@BenHenning
Copy link
Contributor Author

Thanks @gonfunko!

Will need to hold on merging this since the branch needs to be rebased onto v12.

Introduces the necessary base interfaces for representing different
focusable contexts within Blockly. The actual logic for utilizing and
implementing these interfaces will come in later PRs.
@BenHenning BenHenning force-pushed the introduce-focus-system-interfaces branch from f39886f to 3ae422a Compare February 19, 2025 23:08
@BenHenning BenHenning changed the base branch from develop to rc/v12.0.0 February 19, 2025 23:08
@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Feb 19, 2025
@BenHenning
Copy link
Contributor Author

Probably doesn't need a re-review. There was one conflict in core/blockly.ts due to IFlyoutInflater but it was a trivial resolution. The changes look clean and otherwise identical to the previous version that was based off develop.

@BenHenning BenHenning enabled auto-merge February 19, 2025 23:12
They didn't trigger automatically after a force push.
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
2 participants