Fix resizing column logic #41
Conversation
Preview URLsGH Env: preview |
NullVoxPopuli
requested review from
ynotdraw,
camskene,
danwenzel and
nicolechung
NullVoxPopuli
force-pushed
the
fix-resizing-column-logic
branch
from
0288cdd to
bd66b7a
Compare
| /** | ||
| * There can be nothing after the last column | ||
| */ | ||
| if (referenceIndex > visible.length - 1) { |
There was a problem hiding this comment.
Doesn't this need to be:
| if (referenceIndex > visible.length - 1) { | |
| if (referenceIndex >= visible.length - 1) { |
There was a problem hiding this comment.
if there are 5 columns
and reference index is 4 (max), there is no next column -- due to 4 > 5 - 1 being falsey... we hit line 198, and accidentally get a falsey value, which is correct anyway
hm. this is copy and paste from the other implementation, but I'm going to fix this over in: #40
(and then delete this and the original implementation).
I do not like accidentally working. thanks for catching this!
| import * as QUnit from 'qunit'; | ||
| import { module, test, skip } from 'qunit'; | ||
| import { setupRenderingTest } from 'ember-qunit'; | ||
| import { setOwner } from '@ember/application'; | ||
| // @ts-ignore |
There was a problem hiding this comment.
Why the ts-ignore here?
There was a problem hiding this comment.
typed ember team hasn't added types for these yet :(
(and I haven't felt like PRing yet)
It'll be a non problem once TS 4.8 (or whatever officially has types) is the minimum supported version
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Requires #12
(will make the diff actually understandable)
Will be made simpler by: #40
(but that's not required by this work, and can lead to a nice delete a bunch of things PR later