fix(ui): flux sort no longer being overidden by default FE sort #16235
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
ghost
requested review from
desa and
ebb-tide
and removed request for
a team
asalem1
force-pushed
the
flux-query-sort
branch
from
e1cb7e3
to
74599fc
Compare
drdelambre
reviewed
Dec 16, 2019
| return ( | ||
| <div | ||
| style={this.style} | ||
| className={this.class} | ||
| onClick={this.handleClick} | ||
| data-column-index={columnIndex} | ||
| data-row-index={rowIndex} | ||
| {...(rowIndex === 0 ? {'data-testID': `${data}-table-header`} : {})} // conditionally adds test-ID for tableCell header |
There was a problem hiding this comment.
Ya but it’s less that conditionally rendering a component with 1 added prop / adding a testID unnecessarily and sullying the DOM
drdelambre
reviewed
Dec 16, 2019
| } | ||
| const headers = table.data[0] |
There was a problem hiding this comment.
this was the initial default statement set for filtering. While the intention of it is not clear to me, the function of it is to set the default sort to undefined
drdelambre
approved these changes
Dec 16, 2019
alexpaxton
pushed a commit
that referenced
this pull request
Jan 9, 2020
fix(ui): flux sort no longer being overidden by default FE sort
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.
Closes #15980
Problem
The previous implementation was setting a default FE sort for columns if they existed without considering whether the results were already sorted
Solution
Removed the default sort assignment so that values are returned based on the flux query sorting. Added tests to ensure that default values are automatically sorted by
_value