fix: update field editing focus outline style

[microbit-matt-hillsdon]

When editing a field, workspace focus outline does not disappear and the field does not show passive focus outline.

We think that

• Not removing the workspace outline is a definite improvement, it's a considerable visual distraction when moving to a field or dropdown for the focus indicator to disappear
  • Users are unaware of the widget / dropdown div and don't consider focus to have moved out of the workspace
  • When we close dropdown divs when clicking outside the workspace it will further reinforce this impression
• Partial focus is not helpful here
  • It looks weird when you have a normal input focused (you can partly see the dashed line behind the widget). It doesn't provide any useful context in this case.
  • For dropdowns we still think it's more confusing than helpful, but a more balanced judgement.

I originally thought that we needed more CSS classes to implement this but @microbit-grace has managed with what we have.

We're open to just doing this in MakeCode in the short term, but would prefer it in the plugin or at least a review to the effect of checking there are no objections.

CC @BenHenning

Fixes #508
Fixes #586
Part of #645

Demo: https://field-edit-focus-style.blockly-keyboard-experimentation.pages.dev/

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[microbit-matt-hillsdon]

Huh CLA check perhaps because I opened the PR for @microbit-grace? We can fix that tomorrow if needed, I just wanted to write the essay above!

[maribethb]

@microbit-matt-hillsdon there is no email address associated with the PR, maybe you need to check your github settings to make sure there's an email associated with web commits? but opening a PR isn't a commit, so idk. whttps://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address

perhaps making an additional commit to this branch would give it an email address

[microbit-matt-hillsdon]

@microbit-matt-hillsdon there is no email address associated with the PR, maybe you need to check your github settings to make sure there's an email associated with web commits?

I'd already done that bit. Making it public on my profile then rescanning seems to have done the trick.

[BenHenning]

Thanks @microbit-matt-hillsdon! Approving this to unblock and since it seems completely reasonable, but I did have a suggestion for improving the long-term maintainability of this.

If you want to defer the comment, could you please file a follow-up issue to track? I'll merge this when the comment can be addressed (even if it's just a comment), but I'm approving in case someone else needs to merge this sooner.

[BenHenning]

For this & the other CSS: I think this is starting to get quite complicated to follow, and would benefit from at least comments for each new section explaining briefly what it's intending to do (even if some of them can just say 'look at the same selector above for an explanation').

Separately, if there's time it may well be worth adding tests for some or all of these cases. Without explicitly verifying CSS, I suspect it has a decent chance of breaking at some point in the future (and probably being really unclear to whoever breaks it what they should be testing).

[microbit-matt-hillsdon]

Yeah agreed, Grace and I will do both of these but let's do it as a follow up after sorting out how this change relates to #637

I think if I go through it and add more comments now I'll make that harder to sort out whichever way we decide.

Please make a judgement as to whether it's best to merge this now and rework that PR to include this change or hold off for that to land and rework this one.

Raised #645, please feel free to assign to me (if possible).

[maribethb]

@microbit-matt-hillsdon @rachel-fenichel is going to address aaron's comment on #637 since Christopher has gone home for the day, that way we can get that submitted today and unblock this PR.

[microbit-matt-hillsdon]

Thanks!
We've merged in main and moved our changes to the index.ts file.
I've also added comments to the CSS. We'll look at testing the styles in a separate PR.

[microbit-matt-hillsdon]

Back to draft for a mo as we've spotted a selector issue.

[microbit-matt-hillsdon]

Back to draft for a mo as we've spotted a selector issue.

Fixed in 250494c with simplification in 39dbe81 / 3ea518a.

[BenHenning]

Thanks @microbit-matt-hillsdon. The comments make this much clearer.

[BenHenning]

Just waiting on CI but it seems otherwise this can be merged.

[BenHenning]

Going ahead and merging.