Commit 5378c98
authored
feat: Use FocusManager for navigation & state management (#482)
Fixes #142
Fixes #481
This removes nearly all custom focus/blur management and instead leverages `FocusManager` as the source of truth and primary system for focusing/selecting elements.
This largely continues the work in Core starting with RaspberryPiFoundation/blockly#8938 and ending with RaspberryPiFoundation/blockly#8941 in order to bring basic parity in the keyboard navigation plugin using `FocusManager`.
Specifically:
- This replaces all cases where explicit focus/opening logic is needed with `focusNode`/`focusTree` calls via `FocusManager`.
- `Navigation` now infers state based on what's currently focused rather than tracking it separately.
- All of the existing focus/blur response logic has been made redundant due to the inferred navigation mode and focus being tightly coupled with the components themselves.
- The gesture monkey patch seems no longer necessary since, as far as I can tell, focus gets forced back to where it's supposed to be at specific lifecycle moments (which helps undo many of the problems caused by `WorkspaceSvg`'s `markFocused`).
- The passive indicator is no longer necessary now that the `FocusManager`-driven `blocklyPassiveFocus` CSS class exists for styling.
- Note that there's a lot of automatic state synchronizing that has been purposely pushed into Core to simplify the navigation pieces, particularly:
- Blocks will auto-select/unselect themselves based on focus.
- `Toolbox` will automatically synchronize its selected item state with its item that has active focus.
- `Toolbox` and `Flyout` cooperate for automatically closing the `Flyout` at the correct times.
- The new CSS classes are not yet final as the overall strategy for indicating active/passive focus isn't final. There's also additional work needed to ensure selection and active focus can be properly stylized as a combined state, but this is blocked on the completion of RaspberryPiFoundation/blockly#8942.
- `Toolbox` navigation was moved to using its methods directly instead of its event since selection logic now needs to be paired with `focusNode`.
- There are a number of changes in this PR that could probably go into Core, but these can wait for a future PR as they shouldn't be API breaking. Some of the changes include:
- `Toolbox` item selection could auto-focus within `Toolbox` itself, or this could be done as part of `Toolbox`-specific navigation.
- `Flyout` focus could automatically focus its first item if nothing is currently focused.
- For regression testing, the following has been checked:
- 'T' and 'I' do work as expected (ensuring RaspberryPiFoundation/blockly#8933 (review) is no longer an issue).
- Verified #227, #200, #229, #222, and #287 have not regressed. RaspberryPiFoundation/blockly-samples#2498 shouldn't be possible anymore (even with the fix) due to ephemeral focus, and we seem to be matching parity with #326 per the `handleFocus*()` functions, though there may be cases from `handleFocusWorkspace` yet to check.
The test updates were necessary because, at present, clicking does not force focus which means the tests need to rely purely on keys for navigation. I think these changes are actually an improvement since it closer aligns them with keyboard-only usage.
**Current gaps** that may need resolution prior to this PR being mergeable:
- There's a regression with clicking to create a variable closing the flyout that I'm still investigating. This is detailed more in RaspberryPiFoundation/blockly#8941.
- At some point between the latest changes to this branch and the ones in RaspberryPiFoundation/blockly#8941 block movement regressed slightly: finishing a movement gesture now defocuses the workspace entirely (even though this was working earlier).
- There are a lot of inconsistencies between the focus and selection styling before this PR and with this PR. It's unclear how much of this requires actual resolution.
- Much more testing is needed to understand gaps as this changes a significant amount of state handling for the plugin.1 parent c90aae9 commit 5378c98
File tree
16 files changed
+211
-857
lines changed- src
- actions
- test
- webdriverio
- test
16 files changed
+211
-857
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
53 | 54 | | |
54 | 55 | | |
55 | 56 | | |
| 57 | + | |
56 | 58 | | |
57 | 59 | | |
58 | 60 | | |
| |||
67 | 69 | | |
68 | 70 | | |
69 | 71 | | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
76 | 77 | | |
77 | 78 | | |
78 | 79 | | |
| |||
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
103 | | - | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
104 | 107 | | |
105 | 108 | | |
106 | | - | |
107 | | - | |
108 | | - | |
| 109 | + | |
| 110 | + | |
109 | 111 | | |
110 | 112 | | |
111 | 113 | | |
| |||
173 | 175 | | |
174 | 176 | | |
175 | 177 | | |
176 | | - | |
177 | | - | |
178 | | - | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
179 | 196 | | |
180 | 197 | | |
181 | 198 | | |
| |||
221 | 238 | | |
222 | 239 | | |
223 | 240 | | |
224 | | - | |
225 | | - | |
226 | | - | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
227 | 250 | | |
228 | 251 | | |
229 | 252 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
| |||
281 | 282 | | |
282 | 283 | | |
283 | 284 | | |
284 | | - | |
| 285 | + | |
285 | 286 | | |
286 | 287 | | |
287 | 288 | | |
| |||
375 | 376 | | |
376 | 377 | | |
377 | 378 | | |
378 | | - | |
379 | | - | |
380 | | - | |
381 | | - | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
382 | 383 | | |
383 | 384 | | |
384 | 385 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
134 | 135 | | |
135 | 136 | | |
136 | 137 | | |
137 | | - | |
| 138 | + | |
138 | 139 | | |
139 | 140 | | |
140 | 141 | | |
| |||
146 | 147 | | |
147 | 148 | | |
148 | 149 | | |
149 | | - | |
| 150 | + | |
150 | 151 | | |
151 | 152 | | |
152 | 153 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
8 | 13 | | |
9 | 14 | | |
10 | 15 | | |
| |||
29 | 34 | | |
30 | 35 | | |
31 | 36 | | |
32 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
33 | 41 | | |
34 | 42 | | |
35 | 43 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
132 | 133 | | |
133 | 134 | | |
134 | 135 | | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
135 | 139 | | |
136 | 140 | | |
137 | 141 | | |
| |||
159 | 163 | | |
160 | 164 | | |
161 | 165 | | |
| 166 | + | |
| 167 | + | |
162 | 168 | | |
163 | 169 | | |
164 | 170 | | |
| |||
205 | 211 | | |
206 | 212 | | |
207 | 213 | | |
| 214 | + | |
| 215 | + | |
208 | 216 | | |
209 | 217 | | |
210 | 218 | | |
| |||
This file was deleted.
0 commit comments