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

Add id property #123

Closed
jakezatecky opened this issue Jan 22, 2019 · 3 comments
Closed

Add id property #123

jakezatecky opened this issue Jan 22, 2019 · 3 comments

Comments

@jakezatecky
Copy link
Owner

Currently, the node ID used for accessibility purposes in the rendered tree is automatically generated by the library. There may be some use cases where specifying an alternative ID would be preferable.

@jakezatecky
Copy link
Owner Author

Forgot about #116.

@afmfe-iul
Copy link

Hey there, first of all thanks for making this component! 🙂
We have a project with TypeScript, react-testing-library and the latest version of react-checkbox-tree and we would like to mention two problems related to this issue:

  • The README mentions that id is a valid property of <CheckboxTree />, however this simple example doesn't compile:
import React from "react";
import CheckboxTree from "react-checkbox-tree";

export default () => {
  const dummy = () => null;
  return <CheckboxTree id="123" nodes={[]} checked={[]} expanded={[]} onCheck={dummy} onExpand={dummy} />;
};

The error is the following:

No overload matches this call.
  Overload 1 of 2, '(props: Readonly<CheckboxProps>): CheckboxTree', gave the following error.
    Type '{ id: string; nodes: never[]; checked: never[]; expanded: never[]; onCheck: () => null; onExpand: () => null; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<CheckboxTree> & Readonly<CheckboxProps> & Readonly<{ children?: ReactNode; }>'.
      Property 'id' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<CheckboxTree> & Readonly<CheckboxProps> & Readonly<{ children?: ReactNode; }>'.
  Overload 2 of 2, '(props: CheckboxProps, context?: any): CheckboxTree', gave the following error.
    Type '{ id: string; nodes: never[]; checked: never[]; expanded: never[]; onCheck: () => null; onExpand: () => null; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<CheckboxTree> & Readonly<CheckboxProps> & Readonly<{ children?: ReactNode; }>'.
      Property 'id' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<CheckboxTree> & Readonly<CheckboxProps> & Readonly<{ children?: ReactNode; }>'.
  • The other problem we have is validation for what you said here

There may be some use cases where specifying an alternative ID would be preferable.

In short, our testing library and good practices in general expect that a form label must be associated with an input field via id, and since we can't control the id of <CheckboxTree /> we aren't able to follow this pattern properly.

Any updates on this topic would be much appreciated, thanks!

@jakezatecky
Copy link
Owner Author

jakezatecky commented Dec 11, 2019

Hello @afmfe-iul. My apologies for the late reply.

The id property was indeed missing from the types and was fixed in #145 / 28be996. It just never got rolled out with a new npm release. Because this is a breaking bug for some usages and other features are already developed, I went ahead and released v1.6.0, which should fix your issue.

Your comment made me realize to me that I neglected to make id apply to the top-level DOM node (which I resolved in #180).

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

No branches or pull requests

2 participants