Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] add focus style props for non DOM renderers #33

Closed

Conversation

JohnPaulHarold
Copy link

@JohnPaulHarold JohnPaulHarold commented Feb 19, 2021

This PR:

  • adds focusedStyle, focusedLeafStyle, disabledStyle and activeStyle props. These props merge a given style object to the existing style object prop for the inner component.
  • adds the optional chaining babel plugin(https://babeljs.io/docs/en/babel-plugin-proposal-optional-chaining). I needed this while importing the lib locally, via file:../etc. Running npm run build and then importing your forked lib via file:../path/to/lrud wasn't recognising the optional chaining here, https://github.com/jamesplease/lrud/blob/master/src/utils/create-node.ts#L77 This is important as with this fix I can resume creating replicas of the /examples folder, using revas instead
  • tested on the revas renderer, which uses similar styling idioms to React Native

Example

const styles = {
  view: {
    borderWidth: 1,
    borderColor: 'transparent',
    borderStyle: 'solid'
  },
  focused: {
    borderColor: 'magenta'
  }
}

<FocusNode elementType={View} style={styles.view} focusedStyle={styles.focused}/>

Notes

My motivation for writing this was to use the lrud lib with a non ReactDom renderer, in this case revas.

Issues

  • I had difficulties writing a test for the <FocusNode/> component, as to do so would require react and react-dom. Adding those, even if only for a test, seems to include them in the bundle deps and this then causes conflicts with react projects wanting to use lrud. I think the issue is related to the output of the build including *.test.ts{x} files as well, but am not sure, and also could not work out how to make babel not include such files when building.
  • with the above in mind, I buffed up the coverage for the warning() util, which gets a green tick on the pipeline, but is perhaps against the intent of the coverage. Certainly feels like it's gaming it a bit.
  • I've copied some of the examples, modifying them for the alternate renderer, and so far have basic, grid, trap working much like the react-dom equivalents. I will push them to my github repo soon.

@JohnPaulHarold
Copy link
Author

@jamesplease after a bit of a false start, this PR is ready enough for review now.

@JohnPaulHarold
Copy link
Author

JohnPaulHarold commented Mar 5, 2021

closing as, in its current form, it only applies a style to the top of the component, not much use if you want to transform a node inside of the passed element.

As it is, you can still tokenise the classname string and ascertain the focus state that way, so will work that way for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant