-
Notifications
You must be signed in to change notification settings - Fork 380
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
refactor(react-grid): rename the *hiddenColumns getters and props #651
refactor(react-grid): rename the *hiddenColumns getters and props #651
Conversation
@@ -1,2 +1,2 @@ | |||
export const columnChooserItems = (columns, hiddenColumns) => | |||
columns.map(column => ({ column, hidden: hiddenColumns.indexOf(column.name) !== -1 })); | |||
export const columnChooserItems = (columns, hiddenColumnNames) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is not in the plugins directory?
@@ -68,7 +68,7 @@ export default class Demo extends React.PureComponent { | |||
</Toolbar> | |||
<ColumnChooser | |||
columns={columns} | |||
hiddenColumns={hiddenColumns} | |||
hiddenColumnNames={hiddenColumnNames} | |||
onHiddenColumnsChange={this.hiddenColumnsChange} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we also rename onHiddenColumnsChange to onHiddenColumnNamesChange?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we introduce this complexity in naming guidelines? There is a simple rule. The onXXXChange spelling for an event and defaultXXX for a starting value.
|
||
.embedded-demo(column-chooser/uncontrolled) | ||
|
||
### Controlled Mode | ||
|
||
In [controlled mode](controlled-and-uncontrolled-modes.md#controlled-mode), pass the hidden columns to the `TableColumnVisibility` plugin's `hiddenColumns` property and handle the `onHiddenColumnsChange` event to control column visibility externally. | ||
In [controlled mode](controlled-and-uncontrolled-modes.md#controlled-mode), pass the hidden columns to the `TableColumnVisibility` plugin's `hiddenColumnNames` property and handle the `onHiddenColumnsChange` event to control column visibility externally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... pass the hidden column names to the ... hiddenColumnNames
property ?
…ctive into hiddenColumns-hiddenColumnNames
…ctive into hiddenColumns-hiddenColumnNames
BREAKING CHANGES:
We renamed the following TableColumnVisibility plugin's properties to improve the API consistency:
hiddenColumns
->hiddenColumnNames
defaultHiddenColumns
->defaultHiddenColumnNames
onHiddenColumnsChange
->onHiddenColumnNamesChange
The
hiddenColumns
getter has been renamed tohiddenColumnNames
.