Skip to content

Conversation

ewpatton
Copy link

@ewpatton ewpatton commented Jun 4, 2025

Change-Id: Ifcee0a5a30374b0d1e72a24c6c6d538dba16e58f

The basics

The details

Resolves

As discussed at the Blockly Summit, the highlighting behavior of the workspace-search plugin could be improved. This is one attempt at that. It might be worth further tweaking of the CSS attributes.

Proposed Changes

This PR updates the workspace search plugin to desaturate non-matching blocks while supersaturating the selected block to draw more focus to the selection rather than turning the actively highlighted block black. The currently focused block gets saturation +50%, other matching blocks get saturation -50%, and non-matching blocks are completely desaturated.

image

Reason for Changes

Test Coverage

Because changing the coloring logic needed to move the CSS classes around, I updated the test logic for detecting whether a block is actively in the search set.

Documentation

For developers looking to change the behavior, we may want to document how they override the CSS rules defined in plugins/workspace-search/src/css.ts. Anyone overriding the existing CSS rules will need to update their definitions.

Additional Information

Change-Id: Ifcee0a5a30374b0d1e72a24c6c6d538dba16e58f
@ewpatton ewpatton requested a review from a team as a code owner June 4, 2025 21:59
@ewpatton ewpatton requested review from cpcallen and removed request for a team June 4, 2025 21:59
@ewpatton
Copy link
Author

ewpatton commented Jun 4, 2025

I should note I tried using setHighlighted(true) but this didn't seem to work. It's unclear whether this is an interact with the new CSS styles or whether it's an issue in the setHighlighted function itself.

Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

The code changes look good but I'm a bit skeptical about the CSS changes. I may be a bit biased because I have deuteranopia and you happened to pick some green blocks for your example, but I do not think that the proposed approach of adjusting saturation (only) is an improvement from an accessibility point of view.

Can you provide a bit more information about the summit discussion you mentioned, and what the particular problems with the existing approach that you are attempting to solve?

Is there some reason not to focus (select) the active block? To me that seems like the most obviously desirable behaviour. If we did that I'd be less concerned about using saturation as a secondary indication, and would be happy to (e.g.) set all non-found blocks to zero, leaving found blocks at default saturation.

@cpcallen cpcallen assigned ewpatton and unassigned cpcallen Jun 19, 2025
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