Skip to content

Commit

Permalink
fix(settings): resize columns and settings UI fixes
Browse files Browse the repository at this point in the history
Fixed multiple issues with the interaction between the settings UI and the resize columns feature.
Fixed performance issue.

BREAKING CHANGE: dropped cosmoz-omnitable-column-{index} classes
BREAKING CHANGE: dropped visibleColumns
BREAKING CHANGE: dropped disabledColumns
Re #418
  • Loading branch information
cristinecula committed Sep 7, 2021
1 parent 8a58637 commit bb0d29f
Show file tree
Hide file tree
Showing 26 changed files with 169 additions and 175 deletions.
11 changes: 1 addition & 10 deletions cosmoz-omnitable-column-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,10 @@ export const columnMixin = dedupingMixin(base => class extends base { // eslint-
reflectToAttribute: true,
observer: '_hiddenChanged'
},
/**
* Index of this column in the list of displayed columns (excluding disabled/hidden columns).
*/
columnIndex: {
type: Number
},

preferredDropdownHorizontalAlign: {
type: String,
computed: '_computePreferredDropdownHorizontalAlign(columnIndex)'
value: 'right'
},

renderHeader: {
Expand Down Expand Up @@ -260,9 +254,6 @@ export const columnMixin = dedupingMixin(base => class extends base { // eslint-
* Override this in column elements if you need a different default width
*/

_computePreferredDropdownHorizontalAlign(columnIndex) {
return columnIndex === 0 ? 'left' : 'right';
}
getString(item, valuePath = this.valuePath) {
if (valuePath === undefined) {
// eslint-disable-next-line no-console
Expand Down
4 changes: 4 additions & 0 deletions cosmoz-omnitable-column-number.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class OmnitableColumnNumber extends rangeColumnMixin(
type: String,
value: '30px'
},
minWidth: {
type: String,
value: '30px'
},
editWidth: {
type: String,
value: '30px'
Expand Down
4 changes: 2 additions & 2 deletions cosmoz-omnitable-header-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ const
return repeat(columns, column => column.name, column => [
html`<div
class="cell ${ column.headerCellClass } header-cell"
index="${ column.columnIndex }"
?hidden=${ column === groupOnColumn }
title=${ column.title }
for=${ column.name }
>${ column.renderHeader(column) }</div>`,
html`<cosmoz-omnitable-resize-nub
.column=${ column }
index="${ column.columnIndex }"
for=${ column.name }
></cosmoz-omnitable-resize-nub>`
]);
},
Expand Down
5 changes: 2 additions & 3 deletions cosmoz-omnitable-item-row.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class OmnitableItemRow extends repeaterMixin(PolymerElement) {
element.toggleAttribute('editable', !!column.editable);
element.setAttribute('title', this._getCellTitle(column, this.item));
element.setAttribute('class', this._computeItemRowCellClasses(column));
element.setAttribute('index', column.columnIndex);
element.setAttribute('for', column.name);
}

_itemUpdated(changeRecord) {
Expand Down Expand Up @@ -123,8 +123,7 @@ class OmnitableItemRow extends repeaterMixin(PolymerElement) {

_computeItemRowCellClasses(column) {
return 'cell itemRow-cell'
+ (column.cellClass ? ' ' + column.cellClass + ' ' : '')
+ ' cosmoz-omnitable-column-' + column.columnIndex;
+ (column.cellClass ? ' ' + column.cellClass + ' ' : '');
}
}
customElements.define(OmnitableItemRow.is, OmnitableItemRow);
69 changes: 12 additions & 57 deletions cosmoz-omnitable.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,9 @@ import { mixin, hauntedPolymer } from '@neovici/cosmoz-utils';
import { isEmpty } from '@neovici/cosmoz-utils/lib/template.js';
import { useOmnitable } from './lib/use-omnitable';
import './lib/cosmoz-omnitable-settings';
import { columnSymbol } from './lib/normalize-settings';

const PROPERTY_HASH_PARAMS = ['sortOn', 'groupOn', 'descending', 'groupOnDescending'],

normalizeSettings = (columns = [], settings = []) => {
const cols = columns.slice();
for (const setting of settings) {
const idx = cols.findIndex(c => c.name === setting.name);
if (idx < 0) {
continue;
}
cols.splice(idx, 1);
}
return [...settings, ...cols.map(({ name, title, priority }) => ({ name, title, priority }))];
};
const PROPERTY_HASH_PARAMS = ['sortOn', 'groupOn', 'descending', 'groupOnDescending'];

/**
* @polymer
Expand All @@ -74,9 +63,9 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
<div class="header" id="header">
<input class="checkbox all" type="checkbox" checked="[[ _allSelected ]]" on-input="_onAllCheckboxChange" disabled$="[[ !_dataIsValid ]]" />
<cosmoz-omnitable-header-row
columns="[[ visibleColumns ]]"
columns="[[ normalizedColumns ]]"
group-on-column="[[ groupOnColumn ]]"
content="[[ _renderSettings(columns, settings, collapsedColumns) ]]"
content="[[ _renderSettings(normalizedSettings, collapsedColumns) ]]"
>
</div>
<div class="tableContent" id="tableContent">
Expand Down Expand Up @@ -120,7 +109,7 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
<div class="item-row-wrapper">
<div selected$="[[ selected ]]" class="itemRow" highlighted$="[[ highlighted ]]">
<input class="checkbox" type="checkbox" checked="[[ selected ]]" on-input="_onCheckboxChange" disabled$="[[ !_dataIsValid ]]" />
<cosmoz-omnitable-item-row columns="[[ visibleColumns ]]"
<cosmoz-omnitable-item-row columns="[[ normalizedColumns ]]"
selected="[[ selected ]]" expanded="{{ expanded }}" item="[[ item ]]" group-on-column="[[ groupOnColumn ]]">
</cosmoz-omnitable-item-row>
<paper-icon-button class="expand" hidden="[[ isEmpty(collapsedColumns.length) ]]" icon="[[ _getFoldIcon(expanded) ]]" on-tap="_toggleItem"></paper-icon-button>
Expand Down Expand Up @@ -394,25 +383,7 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
},

settings: {
type: Object
},

/**
* List of <b>visible</b> columns.
*/
visibleColumns: {
type: Array,
notify: true,
computed: '_computeVisibleColumns(columns, settings)'
},


/**
* @deprecated
* // TODO: drop with next major release
*/
disabledColumns: {
type: Array,
type: Object,
notify: true
},

Expand Down Expand Up @@ -577,7 +548,7 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
_onColumnEditableChanged(event) {
event.stopPropagation();
const { detail: { column }} = event,
{ visibleColumns: columns } = this;
{ columns } = this;

if (!Array.isArray(columns) || columns.length === 0) {
return;
Expand All @@ -587,8 +558,8 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
if (index < 0) {
return;
}
// TODO: review if this is necessary
this.notifyPath(['visibleColumns', index, 'editable']);

this.columns = [...this.columns];
}

_onKey(e) {
Expand Down Expand Up @@ -757,21 +728,6 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
}
}

_computeVisibleColumns(columns, settings) {
return normalizeSettings(columns, settings)
.flatMap(s => {
if (s.disabled) {
return [];
}
const column = columns.find(c => c.name === s.name);
if (!column) {
return [];
}
return [Object.assign(column, { priority: s.priority ?? column.priority })];
})
.map((c, i) => Object.assign(c, { columnIndex: i }));
}

/**
* Checks if the column setup is valid and logs errors.
* As a separate functions to make testing easier.
Expand Down Expand Up @@ -1403,11 +1359,10 @@ class Omnitable extends hauntedPolymer(useOmnitable)(mixin({ isEmpty }, translat
};
}

_renderSettings(columns, settings, collapsed) {
const that = this;
_renderSettings(normalizedSettings, collapsed) {
return litHtml`<cosmoz-omnitable-settings
.settings=${ normalizeSettings(columns, settings) }
.onSettings=${ s => that.settings = s /* eslint-disable-line no-return-assign */ }
.settings=${ normalizedSettings }
.onSettings=${ this.setSettings }
.collapsed=${ collapsed.map(c => c.name) }
>`;
}
Expand Down
16 changes: 7 additions & 9 deletions lib/compute-layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,33 @@ import { html } from 'haunted';
import { FORCE_FIT, layout } from './layout';

const
_toCss = layout =>
_toCss = (layout, config) =>
layout
.map((width, index) =>
width == null || width === 0
? `cosmoz-omnitable-resize-nub[index="${ index }"], .cell[index="${ index }"]{display:none}`
: `.cell[index="${ index }"]{width: ${ width }px;padding: 0 min(3px, ${ width / 2 }px)}`
? `cosmoz-omnitable-resize-nub[for="${ config[index].name }"], .cell[for="${ config[index].name }"]{display:none}`
: `.cell[for="${ config[index].name }"]{width: ${ width }px;padding: 0 min(3px, ${ width / 2 }px)}`
)
.join('\n');

export const
computeLayout = (_columnConfigs, canvasWidth, numColumns) => {
const columnConfigs = _columnConfigs.map(({ hidden, ...config }) =>
hidden ? { ...config, basis: 0, minWidth: 0, grow: 0 } : config
),
const columnConfigs = _columnConfigs.filter(c => !c.hidden),
totalWidths = columnConfigs.reduce((sum, { basis }) => sum + basis, 0);

if (columnConfigs.length > 1 && totalWidths > canvasWidth) {
// drop a column
return computeLayout(columnConfigs.slice(1), canvasWidth, numColumns);
}

const widths = layout(columnConfigs, Math.max(canvasWidth, totalWidths), FORCE_FIT);
const widths = layout(columnConfigs, canvasWidth, FORCE_FIT);

return widths.reduce((sorted, width, i) => {
sorted[columnConfigs[i].index] = width;
return sorted;
}, new Array(numColumns).fill(undefined));
},
toCss = layout =>
toCss = (layout, config) =>
layout.length === 0
? html`<style>.cell {display: none;}</style>`
: html`<style>${ _toCss(layout) }</style>`;
: html`<style>${ _toCss(layout, config) }</style>`;
12 changes: 6 additions & 6 deletions lib/cosmoz-omnitable-resize-nub.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,33 @@ import { nothing } from 'lit-html';
const
ResizeNub = host => {
useEffect(() => {
let initial = 0;
let initial = 0,
initialWidth = 0;
const
move = ev => {
host.dispatchEvent(new CustomEvent('column-resize', {
bubbles: true,
composed: true,
detail: {
delta: ev.clientX - initial,
newWidth: Math.ceil(initialWidth + ev.pageX - initial),
column: host.column
}
}));
initial = ev.clientX;

},
stop = () => {
document.removeEventListener('pointermove', move);
document.removeEventListener('pointerup', stop);
},
start = ev => {
initial = ev.clientX;
initial = ev.pageX;
initialWidth = host.previousElementSibling.getBoundingClientRect().width;
document.addEventListener('pointermove', move);
document.addEventListener('pointerup', stop);
};
host.addEventListener('pointerdown', start);

return () => host.removeEventListener('pointerdown', start);
}, []);
}, [host.nextElementSibling]);

return nothing;
};
Expand Down
20 changes: 19 additions & 1 deletion lib/cosmoz-omnitable-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,56 +54,72 @@ const settingsStyles = `
}
${ checkbox }
`,

parseIndex = str => {
const idx = parseInt(str, 10);
return isFinite(idx) ? idx : undefined;
},

// eslint-disable-next-line max-lines-per-function
useSettings = host => {
usePosition({ anchor: host.anchor, host });
const { settings, onSettings } = host,

const
{ settings, onSettings } = host,
meta = useMeta({ settings, onSettings });

return {
settings: host.settings,
collapsed: host.collapsed,

onDown: useCallback(e => {
if (!e.target.closest('.sort')) {
return;
}

meta.handle = e.currentTarget;
}, [meta]),

onDragStart: useCallback(e => {
const { target } = e,
index = parseIndex(target.dataset.index);

if (!meta.handle?.contains(target) || index == null) {
return e.preventDefault();
}

meta.handle = null;
e.dataTransfer.effectAllowed = 'move';
e.dataTransfer.setData('omnitable/sort-index', index);
setTimeout(() => target.classList.add('drag'), 0);
target.addEventListener('dragend', e => e.target.classList.remove('drag'), { once: true });
}, [meta]),

onDragEnter: useCallback(e => {
const ctg = e.currentTarget;
if (ctg !== e.target) {
return;
}

e.preventDefault();
e.dataTransfer.dropEffect = 'move';
ctg.classList.add('dragover');
}, []),

onDragOver: useCallback(e => {
e.preventDefault();
e.currentTarget.classList.add('dragover');
}, []),

onDragLeave: useCallback(e => {
const ctg = e.currentTarget;
if (ctg.contains(e.relatedTarget)) {
return;
}

ctg.classList.remove('dragover');
}, []),

onDrop: useCallback(e => {
const from = parseIndex(e.dataTransfer.getData('omnitable/sort-index')),
to = parseIndex(e.currentTarget.dataset.index),
Expand All @@ -116,13 +132,15 @@ const settingsStyles = `
newSettings.splice(to + (from >= to ? 0 : -1), 0, newSettings.splice(from, 1)[0]);
onSettings(newSettings);
}, [meta]),

onToggle: useCallback(e => {
const checked = e.target.checked,
idx = parseIndex(e.target.closest('[data-index]')?.dataset.index),
{ settings, onSettings } = meta,
newSettings = settings.slice(),
setting = settings[idx],
priority = e.target.windeterminate ? settings.reduce((acc, s) => Math.max(acc, s.priority), 0) + 1 : setting.priority;

newSettings.splice(idx, 1, { ...settings[idx], disabled: !checked, priority });
onSettings(newSettings);
}, [meta])
Expand Down
Loading

0 comments on commit bb0d29f

Please sign in to comment.