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.

Sign up for GitHub

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

Move EuiBasicTable's itemId prop out of selection into a top-level prop #830

Merged
merged 9 commits into from
May 16, 2018
Merged

Move EuiBasicTable's itemId prop out of selection into a top-level prop #830

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1f44261
Move EuiBasicTable's itemId prop out of selection into a top-level prop
chandlerprall May 14, 2018
41c67ef
changelog
chandlerprall May 14, 2018
63a4b0d
add generic proptype validator for associating properties
chandlerprall May 15, 2018
9acb46f
added unit test for withRequiredProp
chandlerprall May 15, 2018
4bb9e93
Merge branch 'master' into bug/820-table-ids
chandlerprall May 15, 2018
93efbd7
re-add deleted proptype, commit with_required_prop's snapshots
chandlerprall May 15, 2018
d441047
Merge branch 'master' into bug/820-table-ids
chandlerprall May 15, 2018
b56c9a0
update changelog
chandlerprall May 15, 2018
4734056
tweaks in response to PR feedback
chandlerprall May 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@
- Fixes height calculation error on `EuiAccordion` when it starts loads in an open state. ([#816](https://github.com/elastic/eui/pull/816))
- Added aria-invalid labeling on `EuiFormRow` ([#777](https://github.com/elastic/eui/pull/799))
- Added aria-live labeling for `EuiToasts` ([#777](https://github.com/elastic/eui/pull/777))
- Added aria labeling requirements for `EuiBadge` , as well as a generic prop_type function `requiresAriaLabel` in `utils` to check for it. ([#777](https://github.com/elastic/eui/pull/777)) ([#802](https://github.com/elastic/eui/pull/802))
- Added aria labeling requirements for `EuiBadge` , as well as a generic prop_type function `withRequiredProp` in `utils` to check for it. ([#777](https://github.com/elastic/eui/pull/777)) ([#802](https://github.com/elastic/eui/pull/802))
- Ensure switches’ inputs are still hidden when `[disabled]` ([#778](https://github.com/elastic/eui/pull/778))
- Made boolean matching in `EuiSearchBar` more exact so it doesn't match words starting with booleans, like "truest" or "offer" ([#776](https://github.com/elastic/eui/pull/776))
- `EuiComboBox` do not setState or call refs once component is unmounted ([807](https://github.com/elastic/eui/pull/807) and [#813](https://github.com/elastic/eui/pull/813))
- Added better accessibility labeling to `EuiPagination`, `EuiSideNav`, `EuiPopover`, `EuiBottomBar` and `EuiBasicTable`. ([#821](https://github.com/elastic/eui/pull/821))

**Breaking changes**

- Move `EuiBasicTable`'s `itemId` prop from `selection` to a top-level property ([#830](https://github.com/elastic/eui/pull/830))

## [`0.0.46`](https://github.com/elastic/eui/tree/v0.0.46)

- Added `EuiDescribedFormGroup` component, a wrapper around `EuiFormRow`(s) ([#707](https://github.com/elastic/eui/pull/707))
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/actions/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ export class Table extends Component {
};

const selection = {
itemId: 'id',
selectable: (user) => user.online,
selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined,
onSelectionChange: this.onSelectionChange
Expand Down Expand Up @@ -292,6 +291,7 @@ export class Table extends Component {

<EuiBasicTable
items={pageOfItems}
itemId="id"
columns={columns}
pagination={pagination}
sorting={sorting}
Expand Down
10 changes: 5 additions & 5 deletions src-docs/src/views/tables/basic/props_info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ export const propsInfo = {
required: true,
type: { name: 'object[]' }
},
itemId: {
description: 'describes how to extract a unique ID from each item, used for selections & expanded rows',
required: false,
type: { name: 'string | (item) => string' }
},
compressed: {
description: 'Makes the font and padding smaller for the entire table',
type: { name: 'bool' }
Expand Down Expand Up @@ -88,11 +93,6 @@ export const propsInfo = {
__docgenInfo: {
_euiObjectType: 'type',
props: {
itemId: {
description: 'describes how to extract a unique ID from each item',
required: true,
type: { name: 'string | (item) => string' }
},
onSelectionChanged: {
description: 'A callback that will be called whenever the item selection changes',
required: false,
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/expanding_rows/expanding_rows.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ export class Table extends Component {
};

const selection = {
itemId: 'id',
selectable: (user) => user.online,
selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined,
onSelectionChange: this.onSelectionChange
Expand All @@ -219,6 +218,7 @@ export class Table extends Component {
{deleteButton}
<EuiBasicTable
items={pageOfItems}
itemId="id"
itemIdToExpandedRowMap={this.state.itemIdToExpandedRowMap}
isExpandable={true}
hasActions={true}
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/in_memory/in_memory_selection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ export class Table extends Component {
};

const selection = {
itemId: 'id',
selectable: (user) => user.online,
selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined,
onSelectionChange: (selection) => this.setState({ selection })
Expand All @@ -227,6 +226,7 @@ export class Table extends Component {
<div>
<EuiInMemoryTable
items={this.state.users}
itemId="id"
error={this.state.error}
loading={this.state.loading}
message={this.state.message}
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/mobile/mobile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export class Table extends Component {
};

const selection = {
itemId: 'id',
selectable: (user) => user.online,
selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined,
onSelectionChange: this.onSelectionChange
Expand Down Expand Up @@ -219,6 +218,7 @@ export class Table extends Component {

<EuiBasicTable
items={pageOfItems}
itemId="id"
columns={columns}
pagination={pagination}
sorting={sorting}
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/selection/selection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ export class Table extends Component {
};

const selection = {
itemId: 'id',
selectable: (user) => user.online,
selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined,
onSelectionChange: this.onSelectionChange
Expand All @@ -203,6 +202,7 @@ export class Table extends Component {
{deleteButton}
<EuiBasicTable
items={pageOfItems}
itemId="id"
columns={columns}
pagination={pagination}
sorting={sorting}
Expand Down
19 changes: 11 additions & 8 deletions src/components/badge/badge.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,22 +155,25 @@ EuiBadge.propTypes = {
/**
* Will apply an onclick to icon within the badge
*/
iconOnClick: PropTypes.func,
iconOnClick: EuiPropTypes.withRequiredProp(
PropTypes.func,
'iconOnClickAriaLabel',
'Please provide an aria label to compliment your iconOnClick'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "compliment" -> "complement", here and below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also change "aria label" to "aria-label" to be more specific about the prop's name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prop it's expecting is iconOnClickAriaLabel, not specifically aria-label. The full warning in the console is

Warning: Failed prop type: Property "iconOnClick" was passed without corresponding property "iconOnClickAriaLabel"; Please provide an aria label to complement your iconOnClick
in EuiBadge
in Unknown

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha! That makes sense, thanks.

),

/**
* Aria label applied to the iconOnClick button
*/
iconOnClickAriaLabel: EuiPropTypes.requiresAriaLabel('iconOnClick'),
iconOnClickAriaLabel: PropTypes.string,

/**
* Will apply an onclick to the badge itself
*/
onClick: PropTypes.func,

/**
* Aria label applied to the onClick button
*/
onClickAriaLabel: EuiPropTypes.requiresAriaLabel('onClick'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did onClickAriaLabel prop get deleted? Looks like it is still being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bad Cmd+X operation

onClick: EuiPropTypes.withRequiredProp(
PropTypes.func,
'onClickAriaLabel',
'Please provide an aria label to compliment your onClick'
),

/**
* Accepts either our palette colors (primary, secondary ..etc) or a hex value `#FFFFFF`, `#000`.
Expand Down
34 changes: 17 additions & 17 deletions src/components/basic_table/__snapshots__/basic_table.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,22 +205,6 @@ exports[`EuiBasicTable itemIdToExpandedRowMap renders an expanded row 1`] = `
</EuiTableHeaderCell>
</EuiTableHeader>
<EuiTableBody>
<React.Fragment
key="row_0"
>
<EuiTableRow
isSelected={false}
>
<EuiTableRowCell
align="left"
header="Name"
key="_data_column_name_0_0"
textOnly={true}
>
name1
</EuiTableRowCell>
</EuiTableRow>
</React.Fragment>
<React.Fragment
key="row_1"
>
Expand All @@ -234,7 +218,7 @@ exports[`EuiBasicTable itemIdToExpandedRowMap renders an expanded row 1`] = `
key="_data_column_name_1_0"
textOnly={true}
>
name2
name1
</EuiTableRowCell>
</EuiTableRow>
<EuiTableRow
Expand Down Expand Up @@ -263,6 +247,22 @@ exports[`EuiBasicTable itemIdToExpandedRowMap renders an expanded row 1`] = `
header="Name"
key="_data_column_name_2_0"
textOnly={true}
>
name2
</EuiTableRowCell>
</EuiTableRow>
</React.Fragment>
<React.Fragment
key="row_3"
>
<EuiTableRow
isSelected={false}
>
<EuiTableRowCell
align="left"
header="Name"
key="_data_column_name_3_0"
textOnly={true}
>
name3
</EuiTableRowCell>
Expand Down
Loading