Baseweb: Export and document isFocusVisible utility

Created on 13 May 2020  路  10Comments  路  Source: uber/baseweb

When we wrap our App with { BaseProvider } from 'baseui';
LayersManager invokes initFocusVisible from baseui/esm/utils/focusVisible.

Now when we use,
import { isFocusVisible } from 'baseui/utils/focusVisible';
isFocusVisible becomes faulty as initFocusVisible of baseui/utils/focusVisible was never invoked.
Hence, focusVisible.js is not working as singleton due to different copies provided to internal and external code.

Also, there is no index.js file in utils folder which could've provided esm API to focusVisible.js.

Current Behavior

When import {isFocusVisible} from 'baseui/utils/focusVisible'

  • Click Hello & Hello 2 buttons alternatively in this CodeSandbox.
  • _isFocusVisible_ returns true in both mouse & keyboard modality. Recording

#

When import {isFocusVisible} from 'baseui/esm/utils/focusVisible'

  • Click Hello & Hello 2 buttons alternatively & use the keyboard to navigate in this CodeSandbox.
  • _isFocusVisible_ behavior is correct in both mouse & keyboard modality. Recording

Expected Behavior

when import paths have consistency, i.e, no esm paths,
{BaseProvider} from 'baseui' & {isFocusVisible} from 'baseui/utils/focusVisible',
isFocusVisible should return false when modality is mouse & true when modality is the keyboard as correct initFocusVisible should be invoked.

This can be achieved with proper index.js file in utils folder which will serve esm utils internally like in almost all other cases.

Your Environment

| Tech | Version |
| ------- | ------- |
| Base UI | v9.75.1 |
| React | 16.8.6 |
| browser | Chrome |

"main": "src/index.js", "dependencies": { "baseui": "latest", "react": "16.12.0", "react-dom": "16.12.0", "react-scripts": "3.0.1", "styletron-react": "5.2.7" },

I found a similar issue #3187 .
This comment was very helpful.
But importing just one file (focusVisible.js) from esm folder is not very consistent for the app and would make it hard to convey to the next maintainer the reason behind that.

feature help wanted

All 10 comments

So, I'm trying to create a custom BaseButton which has all the style resets plus the accessibility built-in something similar to material UI.
In case, utils/index.js is not supposed to be exposed, I'll have to write my own custom LayersManager which invoked non esm focusVisible.

I would be more than happy to help create a merge request of it is supposed to be exposed or work on exposing BaseButton with accessibility from inside the button folder itself.

Thanks for the detailed report @stackyism. Can you share a bit more about why a custom BaseButton is required? We have considered focusVisible a private API so far. @tajo, do you have an opinion on if it should be exported or not?

@stackyism - have you confirmed this behavior locally? This looks very similar to an issue with the bundler in CodeSandbox.

If you do get this issue locally with your app, can you tell us a bit more about how your app is bundled?

Also, strangely the first sandbox seems to return the correct true/false states when I tried it out.

Note, here is a reproduction in an alternate sandbox where things also work as expected.

We can / should expose focusVisible. The only reason why it's private is missing documentation.

@chasestarr Thank you so much for your quick response.

Can you share a bit more about why a custom BaseButton is required?

We are trying to make a general clickable component which can adhere to any design and have all accessibility properties.
Trying to achieve this with an amalgamation of style resetting similar to BaseButton from baseui/button/styled-components & handler logic from ButtonBase from material-ui.
So, with our custom BaseButton, we can make any clickable component/area with any design.

I hope this answers your query. Please let me know if any further clarification is needed.

@sandgraham Thank you for your quick response as well.

If you do get this issue locally with your app, can you tell us a bit more about how your app is bundled?

I've been able to reproduce this issue locally in my app.
And for more clarification, I reproduced this with create-react-app as well. Recording
package.json of create-react-app I created with the code from earlier sandbox.

Please let me know if any further clarification is required from my side.

@tajo

We can / should expose focusVisible. The only reason why it's private is missing documentation.

Wow! This sounds great.
I would love to pick this up.

#

Some helpful information required to proceed:

  • What all APIs you'd like to get exposed from baseui/utils folder in its index.js ?
  • Any references to the kind of documentation you're expecting for the same.

What all APIs you'd like to get exposed from baseui/utils folder in its index.js ?

isFocusVisible, handleBlurVisible, initFocusVisible, teardown (should be renamed to teardownVisible probably)

Any references to the kind of documentation you're expecting for the same.

  • I would try to improve flow types and add Typescript types.
  • Explaining why the focus ring is important for a11y.
  • Design issues with the focus ring (mouse clicks should not make it visible)
  • Styling with :focus-visible, the polyfill
  • Why and how we implement this functionality in JavaSript
  • How to use our APIs, what colors to use to match base web, outline vs border (to respect border radius), outline offset to prevent overflows
  • Many examples
  • Should be a standalone page in docs

I suspect this topic is not that well known so it deserves some detailed description. I think we have pretty good strategies that are not used in that many libraries.

@tajo sounds great!
I had done some reading based upon the reference provided in the focusVisible.js file.
Let me share a rough draft addressing all the points you mentioned above by tomorrow.
I would like to pick this up.

Please let me know if you'd like me to follow any guidelines or any more details regarding this.
Thanks! :)

@chasestarr I apologise as I was busy with my own work.
Is it fine if I pick this up over this weekend ?

Thanks.

Was this page helpful?
0 / 5 - 0 ratings