-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[Table] Add column reorder handle when interaction bar enabled #1250
Changes from 22 commits
43049f3
2706ee7
aafe2c1
1e3017c
db7e2e9
3f9880e
1e498e8
6056224
8497731
f5f5334
e81c766
1355ab1
e257c2e
1c80928
82fea25
4fa5c91
9b8a3e7
069245c
78e4e77
82d8cd6
f9d45e4
0d043e5
5fff7e1
fe966e9
2d3de11
84c3b2b
58fdf67
07a062f
3e8988a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,22 +112,27 @@ export class DragReorderable extends React.Component<IDragReorderable, {}> { | |
const { selectedRegions } = this.props; | ||
|
||
const selectedRegionIndex = Regions.findContainingRegion(selectedRegions, region); | ||
if (selectedRegionIndex < 0) { | ||
return false; | ||
} | ||
|
||
const selectedRegion = selectedRegions[selectedRegionIndex]; | ||
if (Regions.getRegionCardinality(selectedRegion) !== cardinality) { | ||
// ignore FULL_TABLE selections | ||
return false; | ||
if (selectedRegionIndex >= 0) { | ||
const selectedRegion = selectedRegions[selectedRegionIndex]; | ||
if (Regions.getRegionCardinality(selectedRegion) !== cardinality) { | ||
// ignore FULL_TABLE selections | ||
return false; | ||
} | ||
|
||
// cache for easy access later in the lifecycle | ||
const selectedInterval = isRowHeader ? selectedRegion.rows : selectedRegion.cols; | ||
this.selectedRegionStartIndex = selectedInterval[0]; | ||
// add 1 to correct for the fencepost | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick -- can you be a little bit more descriptive here? A "because selected interval is inclusive" or "includes its endpoints" or something would be ok; just looking for a why-the-fencepost. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep |
||
this.selectedRegionLength = selectedInterval[1] - selectedInterval[0] + 1; | ||
} else { | ||
// select the new region to avoid complex and unintuitive UX w/r/t the existing selection | ||
this.props.onSelection([region]); | ||
|
||
const regionRange = isRowHeader ? region.rows : region.cols; | ||
this.selectedRegionStartIndex = regionRange[0]; | ||
this.selectedRegionLength = regionRange[1] - regionRange[0] + 1; | ||
} | ||
|
||
const selectedInterval = isRowHeader ? selectedRegion.rows : selectedRegion.cols; | ||
|
||
// cache for easy access later in the lifecycle | ||
this.selectedRegionStartIndex = selectedInterval[0]; | ||
this.selectedRegionLength = selectedInterval[1] - selectedInterval[0] + 1; // add 1 to correct for the fencepost | ||
|
||
return true; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a thing that already exists: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some?v=control ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-.-