Addressing Issue #4349 - Data attributes on Pivot Item

[oengusmacinog-zz]

Pull request checklist

• Addresses an existing issue: Fixes Data attributes on Pivot Item #4349
• Include a change request file using $ npm run change

Description of changes

Added linkNativeProps prop to PivotItem to allow passing native props (data-* & aria-*) to the link/CommandButton element.

Usage is shown in the example:

<Pivot>
  <PivotItem
    headerText='My Files'
    headerButtonProps={ {
      'data-order': 1,
      'data-title': 'My Files Title'
    } }
  >
    <Label className={ exampleStyles.exampleLabel }>Pivot #1</Label>
  </PivotItem>
</Pivot>

The native props passed in the linkNativeProps prop, will become attributes (e.g. data-order='1' data-title='My Files Title') on the PivotItem links <button> tag which is part of the CommandButton component used there.

[dzearing]

This feels a little janky and causes a lot of ambiguity. E.g. If i provide linkText AND linkNativeProps.children, which has precedence?

2 options:

1. What if we just mixed in native attributes, like we do with button?
2. What if we just standardized on a very specific dataAttributes prop?

[oengusmacinog-zz]

As of right now, you can just pass in native attribute mixed, but they don't go on the button they go on the containing div of each item. There are a few points to the choice of how to do this:

1. Removing the native props from the div and moving them to the button could cause compatibility issues if anyone uses them that way.

2. The outer most div is often not the most useful place to add data props, so standardizing that makes little sense.

3. Assuming the 'most important' element in a component is equally janky. There will likely be some disagreement on what is most important, and there still might be more than one element (e.g. an input field and a label element each need data props).

[dzearing]

You're right about that.

linkText should just be "text" like button "text". So following link* naming convention is further going down the inconsistent naming setup here. But... maybe even better:

headerAs?: React.ReactType<IButtonProps>;
headerProps?: IButtonProps might be even better, which includes "text".

<PivotItem headerProps={{ text: 'asdf', data-foo: ... }}>

Thoughts?

[oengusmacinog-zz]

Well keep in mind there are two Parts to the PivotItem. The 'link' it the top part tab you click to view that PivotItem's content below. I think linkButtonProps would probably be best now. I could throw in linkButtonAs with very little extra effort. I think header would be confusing as I'd assume it's a header in the content/body area of the PivotItem right now with linkText being the label text for the tab. Probably better to keep it consistent now, and we can depreciate all link* props later together and change them to tab or header or whatever.

[dzearing]

In the future I still think Pivot is ripe for some adjustment. Thanks for making this change.