fix: Implement alternative highlighting scheme for search#2556
fix: Implement alternative highlighting scheme for search#2556ewpatton wants to merge 1 commit intoRaspberryPiFoundation:mainfrom
Conversation
Change-Id: Ifcee0a5a30374b0d1e72a24c6c6d538dba16e58f
|
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. |
cpcallen
left a comment
There was a problem hiding this comment.
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.
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.
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