Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

576 - column edits with hidden columns #578

Merged
merged 5 commits into from
Sep 14, 2019
Merged

576 - column edits with hidden columns #578

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d30db0
fix #576 - edits while some columns are hidden preserve hidden columns
alexcjohnson Sep 8, 2019
bf0b3b3
fix canceled column name edit
alexcjohnson Sep 8, 2019
9f3b71e
add tests for edit/delete/rename with hidden cols, and canceled name …
alexcjohnson Sep 8, 2019
302caeb
Merge branch 'master' into 576-hidden-edits
alexcjohnson Sep 13, 2019
ccaa2dd
changelog for column edit + hidden_columns fixes
alexcjohnson Sep 13, 2019
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
15 changes: 10 additions & 5 deletions src/dash-table/derived/header/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,23 @@ const doAction = (
action: (
column: IColumn,
columns: Columns,
visibleColumns: Columns,
columnRowIndex: any,
mergeDuplicateHeaders: boolean,
data: Data
) => any,
selected_columns: string[],
column: IColumn,
columns: Columns,
visibleColumns: Columns,
columnRowIndex: any,
mergeDuplicateHeaders: boolean,
setFilter: SetFilter,
setProps: SetProps,
map: Map<string, SingleColumnSyntaxTree>,
data: Data
) => () => {
const props = action(column, columns, columnRowIndex, mergeDuplicateHeaders, data);
const props = action(column, columns, visibleColumns, columnRowIndex, mergeDuplicateHeaders, data);

const affectedColumIds = actions.getAffectedColumns(column, columns, columnRowIndex, mergeDuplicateHeaders);

Expand Down Expand Up @@ -99,7 +101,10 @@ function doSort(columnId: ColumnId, sortBy: SortBy, mode: SortMode, setProps: Se

function editColumnName(column: IColumn, columns: Columns, columnRowIndex: any, setProps: SetProps, mergeDuplicateHeaders: boolean) {
return () => {
setProps(actions.editColumnName(column, columns, columnRowIndex, mergeDuplicateHeaders));
const update = actions.editColumnName(column, columns, columnRowIndex, mergeDuplicateHeaders);
if (update) {
setProps(update);
}
};
}

Expand Down Expand Up @@ -250,7 +255,7 @@ function getter(
null :
(<span
className='column-header--edit'
onClick={editColumnName(column, visibleColumns, headerRowIndex, setProps, mergeDuplicateHeaders)}
onClick={editColumnName(column, columns, headerRowIndex, setProps, mergeDuplicateHeaders)}
>
<FontAwesomeIcon icon='pencil-alt' />
</span>)
Expand All @@ -260,7 +265,7 @@ function getter(
null :
(<span
className='column-header--clear'
onClick={doAction(actions.clearColumn, selected_columns, column, visibleColumns, headerRowIndex, mergeDuplicateHeaders, setFilter, setProps, map, data)}
onClick={doAction(actions.clearColumn, selected_columns, column, columns, visibleColumns, headerRowIndex, mergeDuplicateHeaders, setFilter, setProps, map, data)}
>
<FontAwesomeIcon icon='eraser' />
</span>)
Expand All @@ -272,7 +277,7 @@ function getter(
className={'column-header--delete' + (spansAllColumns ? ' disabled' : '')}
onClick={spansAllColumns ?
undefined :
doAction(actions.deleteColumn, selected_columns, column, visibleColumns, headerRowIndex, mergeDuplicateHeaders, setFilter, setProps, map, data)
doAction(actions.deleteColumn, selected_columns, column, columns, visibleColumns, headerRowIndex, mergeDuplicateHeaders, setFilter, setProps, map, data)
}
>
<FontAwesomeIcon icon={['far', 'trash-alt']} />
Expand Down
30 changes: 16 additions & 14 deletions src/dash-table/utils/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,25 @@ export function getAffectedColumns(column, columns, headerRowIndex, mergeDuplica
);
}

export function clearColumn(column, columns, headerRowIndex, mergeDuplicateHeaders, data) {
const rejectedColumnIds = getAffectedColumns(column, columns, headerRowIndex, mergeDuplicateHeaders);

return {
data: R.map(R.omit(rejectedColumnIds), data)
};
export function clearColumn(column, columns, visibleColumns, headerRowIndex, mergeDuplicateHeaders, _data) {
const {data} = deleteColumn(column, columns, visibleColumns, headerRowIndex, mergeDuplicateHeaders, _data);
return {data};
}

export function deleteColumn(column, columns, headerRowIndex, mergeDuplicateHeaders, data) {
const {groupIndexFirst, groupIndexLast} = getGroupedColumnIndices(
column, columns, headerRowIndex, mergeDuplicateHeaders, columns.indexOf(column)
export function deleteColumn(column, columns, visibleColumns, headerRowIndex, mergeDuplicateHeaders, data) {
const rejectedColumnIds = getAffectedColumns(
column,
visibleColumns,
headerRowIndex,
mergeDuplicateHeaders
);

return {
columns: R.remove(
groupIndexFirst,
1 + groupIndexLast - groupIndexFirst,
columns: R.filter(
col => (rejectedColumnIds.indexOf(col.id) === -1),
columns
),
...clearColumn(column, columns, headerRowIndex, mergeDuplicateHeaders, data),
data: R.map(R.omit(rejectedColumnIds), data),
// NOTE - We're just clearing these so that there aren't any
// inconsistencies. In an ideal world, we would probably only
// update them if they contained one of the columns that we're
Expand Down Expand Up @@ -106,7 +105,7 @@ export function changeColumnHeader(column, columns, headerRowIndex, mergeDuplica
}

const { groupIndexFirst, groupIndexLast } = getGroupedColumnIndices(
column, newColumns, headerRowIndex, mergeDuplicateHeaders, columnIndex
column, newColumns, headerRowIndex, mergeDuplicateHeaders, columnIndex, true
);

R.range(groupIndexFirst, groupIndexLast + 1).map(i => {
Expand All @@ -122,5 +121,8 @@ export function changeColumnHeader(column, columns, headerRowIndex, mergeDuplica

export function editColumnName(column, columns, headerRowIndex, mergeDuplicateHeaders) {
const newColumnName = window.prompt('Enter a new column name');
if (newColumnName === null) {
return null;
}
return changeColumnHeader(column, columns, headerRowIndex, mergeDuplicateHeaders, newColumnName);
}
36 changes: 35 additions & 1 deletion tests/cypress/tests/standalone/column_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ describe(`column, mode=${AppMode.Actionable}, flavor=${AppFlavor.Merged}, ${AppF
DashTable.getHeader(1, 1).within(() => cy.get('span.column-header-name').should('have.html', 'Paris'));
});

it('keeps hidden pieces when deleting a merged column', () => {
cy.get('.column-4 .column-header--hide').click(); // Boston
cy.get('.column-2 .column-header--hide').click(); // Montréal
DashTable.deleteColumnById(0, 'ccc'); // City
cy.get('.show-hide').click();
cy.get('.show-hide-menu-item input').eq(1).click(); // Montréal
cy.get('.show-hide-menu-item input').eq(2).click(); // Boston
cy.get('.column-1 .column-header-name').eq(2).should('have.html', 'Montréal');
cy.get('.column-2 .column-header-name').eq(1).should('have.html', 'Boston');
});

it('can clear column', () => {
DashTable.getFilter(0).click();
DOM.focused.type(`is num`);
Expand Down Expand Up @@ -62,6 +73,29 @@ describe(`column, mode=${AppMode.Actionable}, flavor=${AppFlavor.Merged}, ${AppF
DashTable.getFilter(4).within(() => cy.get('input').should('have.value', ''));
});

it('keeps hidden pieces when clearing a merged column', () => {
// initial state of city columns
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', '1'));
DashTable.getCell(0, 2).within(() => cy.get('.dash-cell-value').should('have.html', '100'));
DashTable.getCell(0, 3).within(() => cy.get('.dash-cell-value').should('have.html', '1'));
DashTable.getCell(0, 4).within(() => cy.get('.dash-cell-value').should('have.html', '2'));
DashTable.getCell(0, 5).within(() => cy.get('.dash-cell-value').should('have.html', '10'));

cy.get('.column-4 .column-header--hide').click(); // Boston
cy.get('.column-2 .column-header--hide').click(); // Montréal
DashTable.clearColumnById(0, 'ccc'); // City
cy.get('.show-hide').click();
cy.get('.show-hide-menu-item input').eq(2).click(); // Montréal
cy.get('.show-hide-menu-item input').eq(4).click(); // Boston

// after clear and re-show -> only Montréal and Boston have data
DashTable.getCell(0, 1).within(() => cy.get('.dash-cell-value').should('have.html', ''));
DashTable.getCell(0, 2).within(() => cy.get('.dash-cell-value').should('have.html', '100'));
DashTable.getCell(0, 3).within(() => cy.get('.dash-cell-value').should('have.html', ''));
DashTable.getCell(0, 4).within(() => cy.get('.dash-cell-value').should('have.html', '2'));
DashTable.getCell(0, 5).within(() => cy.get('.dash-cell-value').should('have.html', ''));
});

it('can hide column', () => {
DashTable.getHeader(0, 0).within(() => cy.get('span.column-header-name').should('have.html', 'rows'));
DashTable.hideColumnById(0, 'rows');
Expand Down Expand Up @@ -313,4 +347,4 @@ describe(`column, mode=${AppMode.Actionable}`, () => {
DashTable.getHeader(1, 2).within(() => cy.get('span.column-header-name').should('have.html', 'France'));
DashTable.getHeader(2, 2).within(() => cy.get('span.column-header-name').should('have.html', 'Paris'));
});
});
});
43 changes: 43 additions & 0 deletions tests/cypress/tests/standalone/edit_headers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,49 @@ describe(`edit, mode=${AppMode.Typed}`, () => {
});
});

describe(`edit headers while some columns are hidden`, () => {
beforeEach(() => {
cy.visit(`http://localhost:8080?mode=${AppMode.Actionable}&flavor=${AppFlavor.Merged}`);
DashTable.toggleScroll(false);
})
it('preserves hidden columns unchanged when editing visible names', () => {
cy.window().then((win: any) => {
cy.stub(win, 'prompt').returns('Chill');
});
// hide 3 columns - one at the start of a merged set, one in the middle, one not in the set
cy.get('.column-8 .column-header--hide').click({force: true});
cy.get('.column-6 .column-header--hide').click({force: true});
cy.get('.column-1 .column-header--hide').click({force: true});
// edit the merged name
cy.get('.dash-header.column-5 .column-header--edit').eq(1).click({force: true});
// re-show the hidden columns
cy.get('.show-hide').click();
cy.get('.show-hide-menu-item input').eq(1).click();
cy.get('.show-hide-menu-item input').eq(6).click();
cy.get('.show-hide-menu-item input').eq(8).click();
// all columns still exist
cy.get('.dash-header.column-9 .column-header-name').should('have.html', 'Temperature-RO');
// the columns that were hidden when the merged name was changed
// still changed name - so they're still all merged
cy.get('.dash-header.column-6[colspan="4"] .column-header-name').eq(1).should('have.html', 'Chill');
});
});

describe('edit headers, canceled edit', () => {
beforeEach(() => {
cy.visit(`http://localhost:8080?mode=${AppMode.SingleHeaders}`);
DashTable.toggleScroll(false);
});
it('preserves column name if edit is canceled', () => {
cy.window().then((win: any) => {
// user presses cancel -> prompt returns null
cy.stub(win, 'prompt').returns(null);
});
cy.get('.dash-header.column-0 .column-header--edit').eq(0).click();
cy.get('.dash-header.column-0 > div > span:last-child').should('have.html', 'rows');
});
});

describe(`edit headers, mode=${AppMode.SingleHeaders}`, () => {
beforeEach(() => {
cy.visit(`http://localhost:8080?mode=${AppMode.SingleHeaders}`);
Expand Down