Skip to content
This repository was archived by the owner on Jun 23, 2026. It is now read-only.

refactor(CSS): move plugin styling from test/index.html to src/index.js#637

Closed
cpcallen wants to merge 7 commits into
RaspberryPiFoundation:mainfrom
cpcallen:refactor/595
Closed

refactor(CSS): move plugin styling from test/index.html to src/index.js#637
cpcallen wants to merge 7 commits into
RaspberryPiFoundation:mainfrom
cpcallen:refactor/595

Conversation

@cpcallen

@cpcallen cpcallen commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator

Resolves

Fixes #595.

Proposed Changes

Move styling of:

  • Comment delete icon
  • Focus indicators
  • Context menu items

from test/index.html to src/index.ts.

Additionally, via RaspberryPiFoundation/blockly#9201, move a fix for the toolbox focus styling from the plugin to core.

Reason for Changes

It should not be necessary to manually add styling to projects using the @blockly/keyboard-navigation plugin in order for the plugin to function as expectd.

Test Coverage

Manually verified that there were no obvious styling differences after making these changes.

Additional Information

Much of the styling added to src/index.ts, particularly the focus-related styling, should be moved to core. Unfortunately because it intersects with the existing styling of rendered elements, it should probably be done by breaking it up and moving it to appropriate places in core/renderers/, rather than just dumping it in core/css.ts, and I'm just not sure where it should all go yet, so in the interest of expediency I am satisfying #595 and will file a separate issue for the move to core.

cpcallen added 6 commits July 2, 2025 13:18
The blocklyToolboxDiv class was renamed blocklyToolbox in
RaspberryPiFoundation/blockly#8647, fixing RaspberryPiFoundation/blockly#8343.

The references in this repository were not updated but are now
evidently obsolete: if we wanted them we'd have fixed them by now.
Also apply only to .blocklyToolbox instead of *
Also move them from html to .injectionDiv.
@cpcallen cpcallen requested a review from a team as a code owner July 3, 2025 00:49
@cpcallen cpcallen requested review from gonfunko and removed request for a team July 3, 2025 00:49
@cpcallen

cpcallen commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

@microbit-matt-hillsdon: Please take a look at this and RaspberryPiFoundation/blockly#9201.

@cpcallen

cpcallen commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator Author

I can't enable auto-squash on this but am happy to have it merged on my behalf once successfully reviewed.

@cpcallen cpcallen changed the title refactor(CSS): move plugin styling from test/index.html to src/index.js. refactor(CSS): move plugin styling from test/index.html to src/index.js Jul 7, 2025
@gonfunko

gonfunko commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

Rather than having this run at load, can the registering be moved into a (static?) method registerKeyboardNavigationStyles() or the like? We've regularly run into issues with accidental re-registering of styles, which IIRC throws if it happens after inject() is called, and also issues with multiple KeyboardNavigation instances.

@rachel-fenichel

Copy link
Copy Markdown
Collaborator

Rather than having this run at load, can the registering be moved into a (static?) method registerKeyboardNavigationStyles() or the like? We've regularly run into issues with accidental re-registering of styles, which IIRC throws if it happens after inject() is called, and also issues with multiple KeyboardNavigation instances.

If I recall correctly, this has to be run at load (or in a static method called on load) because you have to call Css.register before inject.

@gonfunko

gonfunko commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

That's correct, but I think confining it to a static method that needs to be called will avoid the other issues and is helpful on net.

@rachel-fenichel

Copy link
Copy Markdown
Collaborator

Okay, Maribeth asked me to rebase this and make that change, so I'm also assigning it to myself.

@rachel-fenichel

Copy link
Copy Markdown
Collaborator

Replaced by #650 (to complete the change while Christopher is offline).

@cpcallen cpcallen deleted the refactor/595 branch July 10, 2025 08:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

include necessary styles in plugin source code

3 participants