[Table] Better column reordering #1462

cmslewis · 2017-08-17T16:33:08Z

Fixes #1242, Fixes #1284, Fixes #1273, Fixes #1189

Checklist

Include tests
Update documentation

Changes proposed in this pull request:

Improvements to column reordering:

Reorderable columns now always have a reorder handle, even if the interaction bar is disabled.
Columns can now be reordered even when selection is disabled.
Reordering disjoint selections is now much more intuitive.

Reviewers should focus on:

Do you like the visual design of the reorder handle? And the interactions?

Screenshot

Reordering a column with selection disabled:

Reordering a selected column:

Reordering a multi-column selection:

And of course, all of this works when the interaction bar is visible too:

blueprint-bot · 2017-08-17T19:59:12Z

Try again

Preview: documentation | landing | table
Coverage: core | datetime

adidahiya · 2017-08-18T06:26:28Z

Interactions feel great!

Reordering disjoint selections is now much more intuitive.

I didn't notice how unintutive it was earlier. It is certainly better now. Nice work 👍

Since the table now only moves one of the columns in a disjoint selection and disregards the others, is it possible to make it so that only that one columns (the one where you started the drag) is highlighted after you start a drag? At this point:

If that's too much trouble then don't worry about it for now.

adidahiya · 2017-08-18T06:27:45Z

packages/table/src/common/classes.ts

@@ -17,10 +17,12 @@ export const TABLE_CELL_LEDGER_EVEN = "bp-table-cell-ledger-even";
 export const TABLE_CELL_LEDGER_ODD = "bp-table-cell-ledger-odd";
 export const TABLE_COLUMN_HEADER_TR = "bp-table-column-header-tr";
 export const TABLE_COLUMN_HEADERS = "bp-table-column-headers";
+export const TABLE_COLUMN_HEADER_CELL = "bp-table-column-header-cell";
+export const TABLE_COLUMN_HEADER_CELL_HAS_REORDER_HANDLE = "bp-table-column-header-cell-has-reorder-handle";


yikes this is pretty long. I would be cool with simply bp-table-has-reorder-handle and bp-table-has-interaction-bar

blueprint-bot · 2017-08-21T19:36:36Z

Shorter CSS classes

Preview: documentation | landing | table
Coverage: core | datetime

…be reordered

…at will be reordered" This reverts commit bdcc845. Actually, this leads to some buggy behavior with overlapping regions. Will consider this again later.

blueprint-bot · 2017-08-21T20:02:54Z

Revert "[Experimental] On handle mousedown, select only the region that will be reordered"

Preview: documentation | landing | table
Coverage: core | datetime

blueprint-bot · 2017-08-21T23:45:39Z

Square off icon margin

Preview: documentation | landing | table
Coverage: core | datetime

giladgray · 2017-08-22T15:29:54Z

packages/table/src/interactions/_interactions.scss

+  }
+
+  // keep the editable cell input flush with the cell's left border
+  .bp-table-column-name .bp-table-editable-name.pt-editable-text::before {


this is both long and deeply nested 😨
must it be this way?

giladgray · 2017-08-22T15:32:43Z

packages/table/test/columnHeaderCellTests.tsx

@@ -143,22 +143,18 @@ describe("<ColumnHeaderCell>", () => {
    });

    describe("Reorder handle", () => {
-        const REORDER_HANDLE_CLASS = "reorder-handle";
+        const REORDER_HANDLE_CLASS = "bp-table-reorder-handle-target";


prefer not using bp-table- prefix for test-only class. be obvious that this is not a real Table class, just a hook used for these tests. i was wondering if it belongs in Classes, then I found its usage in mount() below.

It's not a test-only class.

...I should use Classes.TABLE_REORDER_HANDLE_TARGET.

giladgray · 2017-08-22T15:33:53Z

packages/table/test/tableTests.tsx

-        let onSelection: Sinon.SinonSpy;
+        const onColumnsReordered: Sinon.SinonSpy = sinon.spy();
+        const onRowsReordered: Sinon.SinonSpy = sinon.spy();
+        const onSelection: Sinon.SinonSpy = sinon.spy();


prob don't need : Sinon.SinonSpy anymore

giladgray · 2017-08-22T15:34:26Z

packages/table/test/tableTests.tsx

        });

        it("Doesn't work on rows if there is no selected region defined yet", () => {
            const table = mountTable({
                isColumnReorderable: true,
                onColumnsReordered,
            });
-            getTableHeader(getColumnHeadersWrapper(table), 0)
+            const headerCell = getHeaderCell(getColumnHeadersWrapper(table), 0);
+            headerCell


revert? this variable doesn't help

…rint into cl/table-reorder-handle-2

blueprint-bot · 2017-08-22T20:07:25Z

Merge branch 'cl/table-reorder-handle-2' of github.com:palantir/blueprint into cl/table-reorder-handle-2

Preview: documentation | landing | table
Coverage: core | datetime

blueprint-bot · 2017-08-22T20:22:54Z

Cleaner test setup now that handle is wider

Preview: documentation | landing | table
Coverage: core | datetime

giladgray

wow this feels good in the preview!
love removing the cell param from all those methods!

cmslewis · 2017-08-23T19:30:12Z

Read through the docs changes with @tgreenwatts. Ready to merge this as soon as the build finishes. 👍

blueprint-bot · 2017-08-23T19:31:11Z

Fix reordering docs

Preview: documentation | landing | table
Coverage: core | datetime

cmslewis added 9 commits August 16, 2017 11:56

Get rid of unneeded TABLE_DRAGGABLE class

3dd948d

Always show reorder handle if column header cell is reorderable

df1ac0b

Add new state classes

1da2288

Enable column reordering in example

9b93cc4

Style the reorder handle

ead53f9

Newly reordered region becomes the only selection

b32bded

Fix reordering in example when selection disabled

a0b269b

Fix editable-cell border in column header

5e096b0

Fix reordering tests

471d5c7

cmslewis added the Package: table label Aug 17, 2017

cmslewis requested review from llorca, themadcreator, adidahiya and gscshoyru August 17, 2017 16:33

cmslewis added 2 commits August 17, 2017 12:02

Try to fix stylelint errors

186cce9

Try again

8cc9c2d

adidahiya reviewed Aug 18, 2017

View reviewed changes

Shorter CSS classes

e2c34f7

cmslewis added 2 commits August 21, 2017 12:52

[Experimental] On handle mousedown, select only the region that will …

bdcc845

…be reordered

Revert "[Experimental] On handle mousedown, select only the region th…

8540bdc

…at will be reordered" This reverts commit bdcc845. Actually, this leads to some buggy behavior with overlapping regions. Will consider this again later.

Square off icon margin

f7509ac

llorca force-pushed the cl/table-reorder-handle-2 branch from 8540bdc to f7509ac Compare August 21, 2017 23:38

giladgray reviewed Aug 22, 2017

View reviewed changes

cmslewis added 2 commits August 22, 2017 12:25

Respond to CR feedback

6c39f1f

Merge branch 'cl/table-reorder-handle-2' of github.com:palantir/bluep…

d576b75

…rint into cl/table-reorder-handle-2

Cleaner test setup now that handle is wider

c754790

giladgray approved these changes Aug 23, 2017

View reviewed changes

Fix reordering docs

5c6725f

tgreenwatts approved these changes Aug 23, 2017

View reviewed changes

cmslewis merged commit f853b4d into master Aug 23, 2017

cmslewis deleted the cl/table-reorder-handle-2 branch August 23, 2017 20:26

cmslewis mentioned this pull request Aug 23, 2017

Prepare Release 1.26.0 #1494

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Table] Better column reordering #1462

[Table] Better column reordering #1462

cmslewis commented Aug 17, 2017 •

edited

Loading

blueprint-bot commented Aug 17, 2017

adidahiya commented Aug 18, 2017

adidahiya Aug 18, 2017

blueprint-bot commented Aug 21, 2017

blueprint-bot commented Aug 21, 2017

blueprint-bot commented Aug 21, 2017

giladgray Aug 22, 2017

giladgray Aug 22, 2017

cmslewis Aug 22, 2017

giladgray Aug 22, 2017

giladgray Aug 22, 2017

cmslewis Aug 22, 2017

blueprint-bot commented Aug 22, 2017

blueprint-bot commented Aug 22, 2017

giladgray left a comment

cmslewis commented Aug 23, 2017

blueprint-bot commented Aug 23, 2017

[Table] Better column reordering #1462

[Table] Better column reordering #1462

Conversation

cmslewis commented Aug 17, 2017 • edited Loading

Fixes #1242, Fixes #1284, Fixes #1273, Fixes #1189

Checklist

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

blueprint-bot commented Aug 17, 2017

Try again

adidahiya commented Aug 18, 2017

adidahiya Aug 18, 2017

Choose a reason for hiding this comment

blueprint-bot commented Aug 21, 2017

Shorter CSS classes

blueprint-bot commented Aug 21, 2017

Revert "[Experimental] On handle mousedown, select only the region that will be reordered"

blueprint-bot commented Aug 21, 2017

Square off icon margin

giladgray Aug 22, 2017

Choose a reason for hiding this comment

giladgray Aug 22, 2017

Choose a reason for hiding this comment

cmslewis Aug 22, 2017

Choose a reason for hiding this comment

giladgray Aug 22, 2017

Choose a reason for hiding this comment

giladgray Aug 22, 2017

Choose a reason for hiding this comment

cmslewis Aug 22, 2017

Choose a reason for hiding this comment

blueprint-bot commented Aug 22, 2017

Merge branch 'cl/table-reorder-handle-2' of github.com:palantir/blueprint into cl/table-reorder-handle-2

blueprint-bot commented Aug 22, 2017

Cleaner test setup now that handle is wider

giladgray left a comment

Choose a reason for hiding this comment

cmslewis commented Aug 23, 2017

blueprint-bot commented Aug 23, 2017

Fix reordering docs

cmslewis commented Aug 17, 2017 •

edited

Loading