Skip to content

Commit

Permalink
[bugfix] Fix Column Sorting (#554)
Browse files Browse the repository at this point in the history
It was possible for columns to get into a bad state based on the
ordering of Hammer event handlers and the click event handler. This PR
fixes it temporarily, but I think this needs further refactoring.

Testing this currently would be very difficult (it would require a lot
of intricate ordering of events) so this PR does not add tests. Locking
us into a series of events that will likely be refactored seems
ill-advised.

Fixes #553
  • Loading branch information
pzuraq authored Jun 16, 2018
1 parent 2eec4cb commit a30d08c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 31 deletions.
48 changes: 18 additions & 30 deletions addon/components/ember-th/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import { closest } from '../../-private/utils/element';
import layout from './template';
import { get } from '@ember/object';

const COLUMN_RESIZE = 0;
const COLUMN_REORDERING = 1;
const COLUMN_INACTIVE = 2;
const COLUMN_INACTIVE = 0;
const COLUMN_RESIZING = 1;
const COLUMN_REORDERING = 2;

/**
The table header cell component. This component manages header cell level
Expand Down Expand Up @@ -149,7 +149,6 @@ export default class EmberTh extends Component {
hammer.add(new Hammer.Press({ time: 0 }));

hammer.on('press', this.pressHandler);
hammer.on('pressup', this.pressUpHandler);
hammer.on('panstart', this.panStartHandler);
hammer.on('panmove', this.panMoveHandler);
hammer.on('panend', this.panEndHandler);
Expand All @@ -161,7 +160,6 @@ export default class EmberTh extends Component {
let hammer = this._hammer;

hammer.off('press');
hammer.off('pressup');
hammer.off('panstart');
hammer.off('panmove');
hammer.off('panend');
Expand Down Expand Up @@ -218,41 +216,33 @@ export default class EmberTh extends Component {
}

pressHandler = event => {
let resizeEnabled = this.get('resizeEnabled');
let reorderEnabled = this.get('reorderEnabled');

let [{ clientX, target }] = event.pointers;

if (resizeEnabled && target.classList.contains('et-header-resize-area')) {
this._columnState = COLUMN_RESIZE;
this.get('columnMeta').startResize(clientX);
} else if (reorderEnabled) {
this._columnState = COLUMN_REORDERING;
}
};

pressUpHandler = () => {
if (this._columnState === COLUMN_RESIZE) {
this.get('columnMeta').endResize();
}

next(() => {
this._columnState = COLUMN_INACTIVE;
});
this._originalClientX = clientX;
this._originalTargetWasResize = target.classList.contains('et-header-resize-area');
};

panStartHandler = event => {
let resizeEnabled = this.get('resizeEnabled');
let reorderEnabled = this.get('reorderEnabled');

let [{ clientX }] = event.pointers;

if (this._columnState === COLUMN_REORDERING) {
if (resizeEnabled && this._originalTargetWasResize) {
this._columnState = COLUMN_RESIZING;

this.get('columnMeta').startResize(this._originalClientX);
} else if (reorderEnabled) {
this._columnState = COLUMN_REORDERING;

this.get('columnMeta').startReorder(clientX);
}
};

panMoveHandler = event => {
let [{ clientX }] = event.pointers;

if (this._columnState === COLUMN_RESIZE) {
if (this._columnState === COLUMN_RESIZING) {
this.get('columnMeta').updateResize(clientX);
this._prevClientX = clientX;
} else if (this._columnState === COLUMN_REORDERING) {
Expand All @@ -262,14 +252,12 @@ export default class EmberTh extends Component {
};

panEndHandler = () => {
if (this._columnState === COLUMN_RESIZE) {
if (this._columnState === COLUMN_RESIZING) {
this.get('columnMeta').endResize();
} else if (this._columnState === COLUMN_REORDERING) {
this.get('columnMeta').endReorder();
}

next(() => {
this._columnState = COLUMN_INACTIVE;
});
next(() => (this._columnState = COLUMN_INACTIVE));
};
}
1 change: 1 addition & 0 deletions config/addon-docs.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env node */
'use strict';

// eslint-disable-next-line
const AddonDocsConfig = require('ember-cli-addon-docs/lib/config');

module.exports = class extends AddonDocsConfig {
Expand Down
2 changes: 1 addition & 1 deletion config/deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

module.exports = function(deployTarget) {
let ENV = {
build: {}
build: {},
// include other plugin configuration that applies to all deploy targets here
};

Expand Down

0 comments on commit a30d08c

Please sign in to comment.