Skip to content

VC-4380 Rewrite Column Moving to improve user experience #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

brifly
Copy link

@brifly brifly commented Apr 15, 2021

This PR rewrites the ColumnMoving feature to improve UX.
The main change is that we only show an indication of where the column will be dropped as we are dragging, and do a single move operation on mouse up. The old implementation moved the column as it was dragged, swapping columns as the drag happened.
This feature was written in TypeScript and as part of the change I've migrated some of the classes to TypeScript (with minimal changes to get it compiling).

@@ -201,6 +201,7 @@ exports.mixin = {
this.behavior.changed();
var oldX = this.hScrollValue;
this.hScrollValue = x;
this.sbHScroller.index = x;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required to update the value in the scroller so the grid doesnt randomly jump back to the old position after a shape change

@@ -1433,7 +1433,7 @@ var defaults = {
* @default
* @memberOf module:defaults
*/
columnGrabMargin: 5,
columnGrabMargin: 10,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it a little easier to drag columns to move them or remove them.
We should change the behavior completely though going forward. This is required right now as both the ColumnSelection and ColumnMoving features depend on the mouse down event - this grab margin effectively splits the header cell into two regions, the top pixels are where the column is grabbed, the bottom is where its selected.

A more sensible approach would be for the ColumnSelection feature only select on the mouse up event (instead of down), and to not select if up event is on a different cell than the down event. That way both the ColumnSelection and ColumnMoving could operate on the whole column header.

@@ -0,0 +1,156 @@
import { IFeature } from "./IFeature";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is doing the same as Feature.js. It allows us to make typescript features that behave the same way as the old js ones.
Eventually the Feature.js will be replaced with this when all Features are migrated to TypeScript.

@@ -32,7 +32,7 @@ var Registry = Base.extend('Registry', {
}

// getClassName defined if new item derived from extend-me
name = name || item.getClassName && item.getClassName();
name = name || item.getClassName && item.getClassName() || item.name;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeScript features use item.name to get the name of the feature

@@ -91,7 +91,7 @@ var ColumnSelection = Feature.extend('ColumnSelection', {
!event.primitiveEvent.detail.isRightClick &&
(
grid.properties.autoSelectColumns ||
event.isHeaderCell && event.mousePoint.y >= 5
event.isHeaderCell && event.mousePoint.y >= grid.properties.columnGrabMargin
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix to use the columnGrapMargin instead of hard coding 5 in here

@aaronbloom aaronbloom merged commit 4481dfc into master Apr 15, 2021
@aaronbloom aaronbloom deleted the VC-4390-Rewrite-Column-Moving branch April 15, 2021 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants