[BUG] Fix cases where manual selection includes unrendered or nonexistent rows #748
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds several failing tests for #747.
Add a function `setupAllRowMeta` that walks all the table's rows and sets up a row meta for each one. This is called lazily in the case where the table has a `selection` that includes some rows that don't yet have rowMetas. This can happen if the selection includes rows that haven't yet rendered. The rowMeta for a row is lazily created when it is rendered, so rows that haven't yet rendered won't have a row meta. This usually is not a problem because in normal user interaction with a table, the user will only interact with rows that are rendered. However, it's possible to programmatically set a selection, and in this case a row in the selection may not yet have been rendered. It's also possible for a `selection` to contain a row without a corresponding rowMeta if that row is not part of the table's rows. This is an invalid selection, and this commit adds an Ember.warning for times when we detect such a case. Fixes #747 Modifies one of the 747 test cases to assert that the warning is triggered when ET encounters a selection with a row that is not part of the table.
bantic
force-pushed
the
bantic/bug/manual-selection
branch
from
10c0062
to
961a5d5
Compare
bantic
changed the title
mixonic
reviewed
Jul 30, 2019
| this.set('selection', rows.slice(-5)); | ||
|
|
||
| await table.rows.objectAt(0).checkbox.click(); | ||
| assert.ok(true, 'no error'); |
There was a problem hiding this comment.
Should this also assert that the row is selected when you scroll to it?
There was a problem hiding this comment.
I'll add an assertion that it is selected, but this row is at the top of the table so will already be in view.
mixonic
approved these changes
Jul 30, 2019
Add `test` condition to warn call in collapse-tree
bantic
force-pushed
the
bantic/bug/manual-selection
branch
from
e5bad9e
to
ea8d909
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modify
CollapseTree:setupAllRowMetathat walks all the table's rows andsets up a row meta for each one. This is called lazily in the case where
the table has a
selectionthat includes some rows that don't yet haverowMetas. This can happen if the selection includes rows that haven't yet
rendered. The rowMeta for a row is lazily created when it is rendered,
so rows that haven't yet rendered won't have a row meta. This usually is
not a problem because in normal user interaction with a table, the user will
only interact with rows that are rendered. However, it's possible to
programmatically set a selection, and in this case a row in the selection
may not yet have been rendered.
mapSelectionToMetato contain the extra complexity of finding the row metas (and calling ofsetupAllRowMetaif needed) for a selectionIt's also possible for a
selectionto contain a row without a correspondingrowMeta if that row is not part of the table's rows. This is an invalid selection,
and this commit adds an Ember.warning for times when we detect such a case.
Fixes #747
Modifies one of the 747 test cases to assert that the warning is triggered
when ET encounters a selection with a row that is not part of the table. The added warning has the id
ember-table.selection-invalid.Adds a
warn-handlerstest helper file that provides a functionregisterTestWarnHandlerthat can be used to capture and assert on runtimeEmber.warnwarnings that happen during a test.