Skip to content

refactor(vocal): Default button mutates outline imperatively from focus/blur handlers #226

Description

@untemps

Summary

_onFocus / _onBlur in Vocal reach into buttonRef.current.style.outline and set the value imperatively. The same component otherwise uses a React-managed style prop. Mixing the two patterns is unnecessary and side-steps React's rendering model.

Current state

```tsx
// src/components/Vocal.tsx:313-323
const _onFocus = useCallback(() => {
if (!className && outlineStyle && buttonRef.current) {
buttonRef.current.style.outline = outlineStyle
}
}, [className, outlineStyle])

const _onBlur = useCallback(() => {
if (!className && outlineStyle && buttonRef.current) {
buttonRef.current.style.outline = 'none'
}
}, [className, outlineStyle])
```

The button already has its style prop computed from className/style/etc. The outline is layered in via direct DOM mutation, which:

  • Bypasses React's reconciliation
  • Requires the ref + manual cleanup
  • Makes the focus state untestable through toHaveStyle without firing real focus events

Proposed improvement

Track focus with a local state and fold the outline into the existing style prop:

```tsx
const [isFocused, setIsFocused] = useState(false)
// ...
style={{
// ...existing props
outline: isFocused && !className && outlineStyle ? outlineStyle : 'none',
}}
onFocus={() => setIsFocused(true)}
onBlur={() => setIsFocused(false)}
```

Or — preferable for a11y — rely on the CSS :focus-visible pseudo-class via a <style> tag or a CSS-in-JS approach, so the outline appears only on keyboard focus.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions