From 1df21a052d30758fe72808377ed18c1875b68e7d Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 10:50:38 +0200 Subject: [PATCH 1/7] replace ifs with switch in DataGridRow --- .../ra-ui-materialui/src/list/DatagridRow.js | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index d3f15fdba87..bcde8380900 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -74,21 +74,22 @@ class DatagridRow extends Component { handleRedirection = (path, event) => { const { basePath, id, push } = this.props; - if (path === 'edit') { - push(linkToRecord(basePath, id)); - return; - } - if (path === 'show') { - push(linkToRecord(basePath, id, 'show')); - return; - } - if (path === 'expand') { - this.handleToggleExpanded(event); - return; - } if (!path) return; - push(path); + switch (path) { + case 'edit': + push(linkToRecord(basePath, id)); + return; + case 'show': + push(linkToRecord(basePath, id, 'show')); + return; + case 'expand': + this.handleToggleExpanded(event); + return; + default: + push(path); + return; + } }; computeColSpan = props => { From 23a9b7793db59d604c298e2f9abf9beff779ff06 Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 10:59:23 +0200 Subject: [PATCH 2/7] use ternary to get path for handleRedirection --- packages/ra-ui-materialui/src/list/DatagridRow.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index bcde8380900..e32fda69879 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -62,13 +62,12 @@ class DatagridRow extends Component { if (!rowClick) return; - if (typeof rowClick === 'function') { - const path = await rowClick(id, basePath, record); - this.handleRedirection(path, event); - return; - } + const path = + typeof rowClick === 'function' + ? await rowClick(id, basePath, record) + : rowClick; - this.handleRedirection(rowClick, event); + this.handleRedirection(path, event); }; handleRedirection = (path, event) => { From a441c2d2229b9988782be8bf95f991704bf90d55 Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 11:13:40 +0200 Subject: [PATCH 3/7] add toggleSelection option on DataGridRow rowClick prop --- docs/List.md | 1 + packages/ra-ui-materialui/src/list/DatagridRow.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/docs/List.md b/docs/List.md index 27ef41ac41e..c7ec0080fa3 100644 --- a/docs/List.md +++ b/docs/List.md @@ -814,6 +814,7 @@ export const PostList = (props) => ( * "edit" to redirect to the edition vue * "show" to redirect to the show vue * "expand" to open the `expand` panel +* "toggleSelection" to trigger the `onToggleItem` function * a function `(id, basePath, record) => path` to redirect to a custom path **Tip**: If you pass a function, it can return `edit`, `show` or a router path. This allows to redirect to either `edit` or `show` after checking a condition on the record. For example: diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index e32fda69879..0d7a9de2bc1 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -85,6 +85,9 @@ class DatagridRow extends Component { case 'expand': this.handleToggleExpanded(event); return; + case 'toggleSelection': + this.handleToggle(event); + return; default: push(path); return; From 1def80736dd11237c4d70ceebbfe788b67e6709d Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 13:58:20 +0200 Subject: [PATCH 4/7] add test for toggleSelection option on DatagridRow --- .../ra-ui-materialui/src/list/DatagridRow.js | 2 +- .../src/list/DatagridRow.spec.js | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 packages/ra-ui-materialui/src/list/DatagridRow.spec.js diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index 0d7a9de2bc1..6e125480f84 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -31,7 +31,7 @@ const sanitizeRestProps = ({ ...rest }) => rest; -class DatagridRow extends Component { +export class DatagridRow extends Component { constructor(props) { super(props); this.state = { diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js new file mode 100644 index 00000000000..e4832ca72f9 --- /dev/null +++ b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js @@ -0,0 +1,28 @@ +import assert from 'assert'; +import React from 'react'; +import { shallow } from 'enzyme'; + +import { DatagridRow } from './DatagridRow'; + +describe('', () => { + const defaultProps = {}; + const event = { preventDefault: () => {}, stopPropagation: () => {} }; + + describe('on click event, depending on rowClick prop', () => { + it("should execute the onToggleItem function if the 'toggleSelection' option is selected", () => { + const onToggleItem = jest.fn(); + + const wrapper = shallow( + + ); + + wrapper.instance().handleClick(event); + + expect(onToggleItem.mock.calls.length).toEqual(1); + }); + }); +}); From b846727ae6bd8a0c0cbdc9d439fe757ee23f3e7a Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 14:28:21 +0200 Subject: [PATCH 5/7] add tests for other options of onRowClick --- .../src/list/DatagridRow.spec.js | 69 ++++++++++++++++++- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js index e4832ca72f9..d121a2a6df2 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js @@ -1,17 +1,54 @@ -import assert from 'assert'; import React from 'react'; import { shallow } from 'enzyme'; +import { linkToRecord } from 'ra-core'; import { DatagridRow } from './DatagridRow'; describe('', () => { - const defaultProps = {}; + const defaultProps = { + id: 15, + basePath: '/blob', + }; + const event = { preventDefault: () => {}, stopPropagation: () => {} }; describe('on click event, depending on rowClick prop', () => { + it("should redirect to edit page if the 'edit' option is selected", () => { + const push = jest.fn(); + const wrapper = shallow( + + ); + + wrapper.instance().handleClick(event); + expect(push.mock.calls).toEqual([ + [linkToRecord(defaultProps.basePath, defaultProps.id)], + ]); + }); + + it("should redirect to show page if the 'show' option is selected", () => { + const push = jest.fn(); + const wrapper = shallow( + + ); + + wrapper.instance().handleClick(event); + expect(push.mock.calls).toEqual([ + [linkToRecord(defaultProps.basePath, defaultProps.id, 'show')], + ]); + }); + + it("should change the expand state if the 'expand' option is selected", () => { + const wrapper = shallow( + + ); + expect(wrapper.state('expanded')).toBeFalsy(); + + wrapper.instance().handleClick(event); + expect(wrapper.state('expanded')).toBeTruthy(); + }); + it("should execute the onToggleItem function if the 'toggleSelection' option is selected", () => { const onToggleItem = jest.fn(); - const wrapper = shallow( ', () => { expect(onToggleItem.mock.calls.length).toEqual(1); }); + + it('should redirect to the custom path if onRowClick is a string', () => { + const path = '/foo/bar'; + const push = jest.fn(); + const wrapper = shallow( + + ); + + wrapper.instance().handleClick(event); + expect(push.mock.calls).toEqual([[path]]); + }); + + it('should evaluate the function and redirect to the result of that function if onRowClick is a custom function', async () => { + const push = jest.fn(); + const customRowClick = () => '/bar/foo'; + const wrapper = shallow( + + ); + + await wrapper.instance().handleClick(event); + expect(push.mock.calls).toEqual([['/bar/foo']]); + }); }); }); From 8bdefedc82841254d3160153f6165f591ffc218f Mon Sep 17 00:00:00 2001 From: fargito Date: Thu, 23 May 2019 14:43:38 +0200 Subject: [PATCH 6/7] add test for closing the expand on DatagridRow --- packages/ra-ui-materialui/src/list/DatagridRow.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js index d121a2a6df2..e6b555d6831 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js @@ -45,6 +45,9 @@ describe('', () => { wrapper.instance().handleClick(event); expect(wrapper.state('expanded')).toBeTruthy(); + + wrapper.instance().handleClick(event); + expect(wrapper.state('expanded')).toBeFalsy(); }); it("should execute the onToggleItem function if the 'toggleSelection' option is selected", () => { From 88bbe1b282432ce1aa40dadb28681e9091a71e6c Mon Sep 17 00:00:00 2001 From: fargito Date: Fri, 24 May 2019 09:53:58 +0200 Subject: [PATCH 7/7] refactor tests and make a cleaner switch default --- .../ra-ui-materialui/src/list/DatagridRow.js | 4 +--- .../src/list/DatagridRow.spec.js | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index 6e125480f84..810f4c14f31 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -73,8 +73,6 @@ export class DatagridRow extends Component { handleRedirection = (path, event) => { const { basePath, id, push } = this.props; - if (!path) return; - switch (path) { case 'edit': push(linkToRecord(basePath, id)); @@ -89,7 +87,7 @@ export class DatagridRow extends Component { this.handleToggle(event); return; default: - push(path); + if (path) push(path); return; } }; diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js index e6b555d6831..b4fd923037b 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.spec.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.spec.js @@ -10,9 +10,9 @@ describe('', () => { basePath: '/blob', }; - const event = { preventDefault: () => {}, stopPropagation: () => {} }; + const event = { preventDefault: () => { }, stopPropagation: () => { } }; - describe('on click event, depending on rowClick prop', () => { + describe('rowClick', () => { it("should redirect to edit page if the 'edit' option is selected", () => { const push = jest.fn(); const wrapper = shallow( @@ -90,5 +90,19 @@ describe('', () => { await wrapper.instance().handleClick(event); expect(push.mock.calls).toEqual([['/bar/foo']]); }); + + it('should not call push if onRowClick is falsy', () => { + const push = jest.fn(); + const wrapper = shallow( + + ); + + wrapper.instance().handleClick(event); + expect(push.mock.calls.length).toEqual(0); + }) }); });