Allow data-attributes for all components

[dstoyanoff]

What problem does this feature solve?

Hello, we are using antd in my team and we love it. In the process of our automated testing, we need to add custom html attributes to the DOM elements, so we can easily find them on the page. However most of the components do not respect custom attributes, thus not rendering them. Is it possible that such thing is implemented?

What does the proposed API look like?

Allow rendering ...rest properties, passed to a component.

[yesmeck]

Related #8305

[yesmeck]

All components accept className, you can add unique className to component, then find them by className.

[apieceofbart]

@yesmeck using className is workaround - it pollutes the markup, can potentially break the app if someone uses wrong class name etc. The standard way would be to allow for data attributes to be passed.
Curious: why not allow to pass any custom attributes?

[filipkowal]

@yesmeck Select does not accept className neither.

[afc163]

@filipkowal https://codesandbox.io/s/4xw97z4jyw It works

[sander-vink]

We could really use this feature as well. Abusing the className property is not a good idea, and we would very much like to avoid that workaround.

[matthewlein]

I'm going to start work on PRs to allow data-* attributes for many of the components unless there are are objections from the maintainers. @yesmeck @afc163 Any issues with that? Would you want them in one PR for all, or each component at a time?

[matthewlein]

Planning on something like this matthewlein@335807b

[apieceofbart]

@matthewlein why not allow passing any attributes?

[matthewlein]

@apieceofbart Yea, my preference would be to pass everything, but I opted for the more conservative since I don't own this project.

Ideally, I'd do something like

const { all, props, we, want, ...otherProps } = this.props;
<Component {...otherProps} />

And now that I've looked at the codebase more, there are a fair amount of examples like that:

...
let checkboxProps: CheckboxProps = { ...restProps };
...
const divProps = omit(others, [
  'onTabChange',
]);

So maybe I will switch to that, it should cover data-*, aria-*, and the unknown wild future of attributes.

Ultimately, I just want to do the one that will get merged though 😅.

[apieceofbart]

Yeah, I understand you totally, just a thought that data- isn't that special and there are other cases. I think you'll have to extend (if I'm correct) every props interface to allow data- props, you might as well do that for any custom props. Thanks for working on that!

[matthewlein]

True, types are where things could get a bit tricky. I realize now that the demos .md are not using typescript...I'll have to review that.

You can allow any string prop with [key: string]: any; but that allows any string prop...

aria-* attributes are predefined, so they are easier to include via https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3b8b13474ed4e489c95702d0a337598573428970/types/react/index.d.ts#L991-L1176

[apieceofbart]

hmmm but react allows any dom attribute, right? shouldn't this be in the types as well?
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html

[matthewlein]

@apieceofbart that article deals with jsx's handling of props, but that won't play into ant's component prop type-checking through typescript. Fortunately it seems to Just Work™. TS does not error on data-* or aria-* props. Other random props will error, as expected.

[matthewlein]

With #10922, the following list of components should be passing data-*, aria-*, and role props to their elements or rc-components. There are PRs open on many of the underlying rc-components, and when those are merged this whole list should work directly, or with a workaround.

Alert
Button
Card
Checkbox
DatePicker -> rc-calendar
  The date input -> https://github.com/ant-design/ant-design/pull/10922
    The rc-calendar date picker widget is too complicated to pass through a single props, it would require something complicated like many popupProps, popupProps.inputProps, etc
Form
Icon
Input
InputNumber -> rc-input-number -> https://github.com/react-component/input-number/pull/107
Menu
  Menu.Item
Message -> works by passing JSX to content
Modal -> https://github.com/ant-design/ant-design/pull/10922
  <Modal wrapProps={{
    'data-test': 'test-id'
  }} />
  just needed update ModalProps with `wrapProps?: any;`
Pagination -> rc-pagination -> https://github.com/react-component/pagination/pull/135
Radio
  Radio.Button
Select -> rc-select -> https://github.com/react-component/select/pull/305
  Select.Option
Spin
Table -> rc-table -> https://github.com/react-component/table/pull/211
  Can also accomplish with custom table component
  class MyTable extends React.Component<any, {}> {
    public render() {
      return (
        <table data-test="Table">
          {this.props.children}
        </table>
      )
    }
  }
  <Table columns={columns} dataSource={data} components={{table: MyTable}} />
Tooltip -> rc-tooltip
  works by passing in react element to title
  <Tooltip title={<span data-test="tooltipContent">tooltip content</span>}>
TreeSelect -> rc-tree-select -> https://github.com/react-component/tree-select/pull/112
  TreeNode -> rc-tree -> https://github.com/react-component/tree/pull/172

[liangpure]

I develop a babel-plugin to auto add test className.
babel-plugin-antd-test-class.
if could use data-*, It will be a better choice.

[DylanVann]

Currently I'm finding most Ant Design components work well with data-test-id. notification does not though.

[afc163]

You could feel free to send PR to us for rest components which do not support yet.

[matthewlein]

@DylanVann @afc163 I made that PR to rc-notification on Jun 20...4 months ago 😢
react-component/notification#43

[afc163]

@matthewlein Could you resolve the conflict and I will handle it ASAP.

[DylanVann]

@afc163 I rebased that PR: react-component/notification#54

[hzaben81]

It seems that Tabs.TabPane does not render data-tid. Could you fix this as well?

[hzaben81]

It seems you can do this for tabs pane

  <Tabs.TabPane
    tab={(
      <span data-tid="...">
        {ttitle}
      </span>
    )}
  >

And it will render the data-tid with the text of the tab

[momesana]

All components accept className, you can add unique className to component, then find them by className.

That advice amounts to abusing an attribute that is highly volatile, subject to change and above all meant for styling as a means to query elements. There is a reason data-attributes exist and they also come in handy for ui-testing purposes: https://docs.cypress.io/guides/references/best-practices.html#Selecting-Elements.

It would be really nice and would spare us a lot of workarounds if antd passed unmatched props (...otherProps) down to it's container elements or children.

We worked around this by writing a HOC that wraps the Component in a div/span etc. which correctly handle data-attributes.

[michelmeeuwissen]

<Modal
            title={title}
            visible={true}
            closable={true}
            onCancel={() => onClose()}
            destroyOnClose={true}
            data-testid={'dummy'}  //Does not work
            className={'dummy'} /> //Work
        >

antd 3.26.11
Going to use className for now as a workaround

[RavilM]

Up

[luco]

Up

[YuvarajPandiyan]

https://codesandbox.io/s/4xw97z4jyw

Adding an extra span element will create a problem so slowness unnaseoerry element creation.