Sorting sorts ascending first and displays correct icon#164
Sorting sorts ascending first and displays correct icon#164valentijnnieman merged 12 commits intomasterfrom
Conversation
|
|
||
| isBackEnd(value) { | ||
| return ['be', false].indexOf(value) !== -1; | ||
| } |
There was a problem hiding this comment.
This isn't really part of the issue this PR fixes, but it's a nice fix anyway. Note that the way we have our webpack and babel configs set up now, using arrow functions as class methods throws an error upon building, hence the .bind(this) syntax, which we should get rid of at some point.
There was a problem hiding this comment.
See comment above in metadata.json, otherwise 👍
| DashTable.getSelect(0).within(() => cy.get('input').click()); | ||
| DashTable.getSelect(0).within(() => cy.get('input').should('be.checked')); | ||
| cy.get('tr th.column-0 .sort').last().click({ force: true }); | ||
| cy.get('tr th.column-0 .sort').last().click({ force: true }).click({ force: true }); |
There was a problem hiding this comment.
Double-click to sort twice so the values actually change
There was a problem hiding this comment.
Right, this is the only case where it matters throughout the existing tests as here the derived_viewport_data was still in the same order as the original data.
chriddyp
left a comment
There was a problem hiding this comment.
It seems pretty crazy that this small number of changes would've caused such a big diff in the bundle. Are we missing anything to have reproducable builds, like:
- A package-lock ?
- Stricter node/npm versions?
|
@chriddyp I included the test bundle, which I didn't know was different, my mistake. Rebuild it, should be good now! |
|
|
||
| ## RC7 - Sort ascending on first click | ||
| - Sorts ascending when first clicked | ||
| - Flips icons displayed so that they are pointing up on ascending and down on descending. No newline at end of file |
There was a problem hiding this comment.
Include a link to the issue here since we have it.
dash_table/metadata.json
Outdated
| ], | ||
| "returns": null | ||
| } | ||
| ], |
There was a problem hiding this comment.
Ah! I think that's exactly why I defined this methods in the render function originally!
Since they do not require 'this' they could be moved outside the class entirely, solving this issue as well.
Marc-Andre-Rivet
left a comment
There was a problem hiding this comment.
- merge with master
- rework isFrontEnd / isBackEnd change so as to keep metadata.json file clean
Closes #118 - this flips the icons so that they display down-arrow on descending, and up arrow on ascending, and selects ascending sort at first click.