component-driven / ui Goto Github PK

I discovered a strange behavior in the FocusWithin component when I was building a custom input component. When I clicked outside of the wrapper element of the custom inputs, the lastly activated input kept the focus style.

After that, I tried one of the examples from the documentation in a newly created react app and the FocusWithin component behaved the exact same that I experienced earlier.

Reproduction steps:

1. Create a new react app using the create-react-app command.
2. Paste the attached code snippet to the App.js file (you can find it at the bottom of the issue).
3. Start the application.
4. Click into one of the inputs.
5. Try to click outside of the form element.

Current result: If you click outside of the form horizontally, it works fine, because the area is in the wrapper element of the form, but if you click outside vertically, that space is out of the wrapper component, so the form will keep the focused state.

Expected result: The form should lose the focus that when the user clicks outside of it.

import React from 'react';
import { FocusWithin } from '@component-driven/react-focus-within';

function App() {
  return (
    <div className="App">
      <FocusWithin
        onFocus={() => {
          console.log('Received focus')
        }}
        onBlur={() => {
          console.log('Lost focus')
        }}
      >
        {({ focused, getRef }) => (
          <form style={{ margin: '100px auto', width: '300px' }}>
            <fieldset
              ref={getRef}
              style={{ borderColor: focused ? 'blue' : '#999' }}
            >
              <div>
                <label htmlFor="input1">First input</label>
                <input
                  id="input1"
                  type="text"
                  placeholder="Click to activate first input"
                />
              </div>

              <div>
                <label htmlFor="input2">First input</label>
                <input
                  id="input2"
                  type="text"
                  placeholder="Use Tab to activate next input"
                />
              </div>

              <button type="submit">Submit</button>

              <p style={{ color: focused ? 'danger' : 'text' }}>
                {focused ? 'Focus is inside' : 'No focus here'}
              </p>
            </fieldset>
          </form>
        )}
      </FocusWithin>
    </div>
  );
}

export default App;

The way vanilla-extract handles selectors is a bit different from other CSS-in-JS solutions and it fails to work with the library now. In order to make it work with it, we need to switch to matching the pseudo-selector in the document.styleSheets and then narrow it down to the element somehow.

Actual

import { FocusWithin } from @component-driven/react-focus-within

Expected

import { FocusWithin } from "@component-driven/react-focus-within"

Hey Andrey, I love this component!

I was wondering if there is any specific reason why children is a function. IMHO the state could be lifted up since anyway we get to register onFocus and onBlur. A minimal API like this https://codesandbox.io/s/vq5vzmxxrl would work as a baseline to build more complex ones.

FWIW it is probably better to avoid preventDefault/stopPropagation since other components (ancestors) might rely on those events.

Maybe instead of generating a hash, we could use a replace to replace invalid chars:

const generatedClassName = selector.replace(':', '-')

Currently, if a user has className prop on the component, it will be overwritten.

Even better solution would be to go away from implicit cloneElement under the hood and expose the hook or render props.

Let's display WCAG rating like:

If the ref wasn't passed to the component (for example, it doesn't implement React.forwardRef), when it fails in runtime with

Cannot read properties of null (reading 'classList')

Currently, the component does cloneChildren which is a very implicit and can lead to bugs like #58. It would be easier to implement it as a hook and let folks assign props to their components themselves.

🚨 The automated release from the master branch failed. 🚨

I recommend you give this issue a high priority, so other packages depending on you could benefit from your bug fixes and new features.

You can find below the list of errors reported by semantic-release. Each one of them has to be resolved in order to automatically publish your package. I’m sure you can resolve this 💪.

Errors are usually caused by a misconfiguration or an authentication problem. With each error reported below you will find explanation and guidance to help you to resolve it.

Once all the errors are resolved, semantic-release will release your package the next time you push a commit to the master branch. You can also manually restart the failed CI job that runs semantic-release.

If you are not sure how to resolve this, here is some links that can help you:

• Usage documentation
• Frequently Asked Questions
• Support channels

If those don’t help, or if this issue is reporting something you think isn’t right, you can always ask the humans behind semantic-release.

No npm token specified.

An npm token must be created and set in the NPM_TOKEN environment variable on your CI environment.

Please make sure to create an npm token and to set it in the NPM_TOKEN environment variable on your CI environment. The token must allow to publish to the registry https://registry.npmjs.org/.

Good luck with your project ✨

Your semantic-release bot 📦🚀

The two values transparent and currentColor are valid css color values and therefore, the SmartWatch component should not fail with a runtime error trying to parse them.

example code:

const calculatedTheme = {
    ...theme,
    colors: {
        ...theme.colors,
        transparent: 'transparent',
        current: 'currentColor',
    },
}

return <ThemeColors theme={calculatedTheme} />

without those two values, it works fine

Subj

If the container node got unmounted when the check

https://github.com/okonet/react-simple-focus-within/blob/853d04494ec4ee28140f3c53fd580d9b8de0d7f6/src/FocusWithin.js#L132

will lead to a TypeError Cannot read property 'contains' of null

Now the internal implementation is something different. Maybe we should change the name too.

Only with hex