Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

issue175 editable column#182

Merged
Marc-Andre-Rivet merged 8 commits intomasterfrom
3.1-issue175-editable-column
Oct 30, 2018
Merged

issue175 editable column#182
Marc-Andre-Rivet merged 8 commits intomasterfrom
3.1-issue175-editable-column

Conversation

@Marc-Andre-Rivet
Copy link
Copy Markdown
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Oct 29, 2018

This fixes #175

  • Change isEditable logic so that a cell is editable/not editable when the cell 'isEditable' property is true/false, and use table 'isEditable' prop if undefined.

  • additional units tests for the editable logic

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 29, 2018 20:43 Inactive
editable: boolean,
column: IVisibleColumn
): boolean => editable && (column.editable === undefined || column.editable); No newline at end of file
editableColumn: boolean | undefined
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing the dependency on the column structure here -- we just care about the editable props

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a bit nitpicky, but perhaps naming it columnIsEditable is better, so that it's more clear that it's a boolean and not some sort of column object?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will update to isEditableTable and isEditableColumn

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 29, 2018 20:46 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 29, 2018 23:39 Inactive
* Subscribe to [https://github.com/plotly/dash-table/issues/175](https://github.com/plotly/dash-table/issues/175)
* for more information.
* If the column-level `editable` flag is set it overrides
* the table-level `editable` flag for that column.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updating documentation for table and column editable property

Copy link
Copy Markdown
Contributor

@valentijnnieman valentijnnieman left a comment

Choose a reason for hiding this comment

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

One small suggestion, otherwise looks good to me!

editable: boolean,
column: IVisibleColumn
): boolean => editable && (column.editable === undefined || column.editable); No newline at end of file
editableColumn: boolean | undefined
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a bit nitpicky, but perhaps naming it columnIsEditable is better, so that it's more clear that it's a boolean and not some sort of column object?

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 30, 2018 19:48 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 30, 2018 19:54 Inactive
isEditable(editable, columns[active_cell[1]]) &&
!isMetaKey(e.keyCode)
) {
// setProps({ is_focused: true });
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@valentijnnieman While merging, came across this little nugget of code that does nothing since the navigation work. Removing.

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-182 October 30, 2018 20:14 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

column level editable to take precendence over table level editable?

3 participants