Focusing into a focus zone from detached focus does not use provided `focusInStrategy` function about behaviors

[rezrah]

Hi @iansan5653 - thanks for filing this bug report. We're going to take a closer look at this in the Primer React team over the next week, but unfortunately I can't commit to a resolution date just yet.

At this time, we don't have high confidence that a fix will not create downstream regressions in Primer React. This is because useFocusZone, which many components in Primer React rely on does not have adequate test coverage. To avoid these risks, we want to be a bit more cautious with this update.

Our plan is currently:

1. Backfill the test coverage for useFocusZone in PRC repo (0 coverage)
2. Backfill test coverage for focusZone.ts in this repo (currently <80% coverage)
3. Investigate / remediate reported bug

We're being extra cautious as we've recently seen various ad-hoc bugfixes ship, that created regressions in their dependencies and required rollback.

Did you already have a solution in mind for point 3? If so, please feel free to put a PR up for it, or we can take a look at it once we've resolved the first two points.

[iansan5653]

Did you already have a solution in mind for point 3?

I do, but it is opinionated - I don't know if your team will want to go in the same direction.

In my opinion the best solution would be to just forward the behavior by passing null to the callback. This would require a breaking change to the types though - the type of focusInStrategy would have to change to add null to the function arg type:

- focusInStrategy?: 'first' | 'closest' | 'previous' | ((previousFocusedElement: Element) => HTMLElement | undefined)
+ focusInStrategy?: 'first' | 'closest' | 'previous' | ((previousFocusedElement: Element | null) => HTMLElement | undefined)

Anywhere consuming code is providing a function they'll have to handle the null case. I think this would be a pretty safe change from a testing perspective on this end since we'd just be forwarding the behavior downstream.

[github-actions]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.