Skip to content
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

COMPASS-564: Indexes Tab order gets reversed on create and delete index #695

Merged
merged 4 commits into from Dec 18, 2016

Conversation

pzrq
Copy link
Contributor

@pzrq pzrq commented Dec 16, 2016

Needed to resolve issues with master and COMPASS-541 progress.

@@ -580,9 +579,9 @@ function addClickCommands(client) {
/**
* Click on the name header in the index table.
*/
client.addCommand('clickIndexTableNameHeader', function() {
client.addCommand('clickIndexTableNameHeader', function(columnName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just change this command name now to clickIndexTableHeader since we are providing the column name and not specifically clicking on the name header.

@@ -29,7 +29,9 @@ const SortIndexesStore = Reflux.createStore({
*/
loadIndexes(indexes) {
this.indexes = indexes;
this.sortIndexes(DEFAULT);
this.sortField = DEFAULT;
this.sortOrder = ASC;
Copy link
Member

@durran durran Dec 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand from the ticket, we are expecting the sorting to not change from the existing sorting when adding an index? If that is the case, then I think these 2 lines need to change to:

this.sortField = this.sortField || DEFAULT;
this.sortOrder = this.sortOrder || ASC;

This way, any handling of indexes being loaded would retain the current sorting paramters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even better, I believe those 2 lines can move into the init function... Then load would never have worry about changing them.

@durran
Copy link
Member

durran commented Dec 17, 2016

I added code based off my comments and pushed to the branch.

pzrq and others added 4 commits December 18, 2016 21:00
Should make the tests pass deterministically.
It should always be in the reversed order at this point, because the index list is incorrectly toggling the default value.
If the default “Name and Definition” is selected, which was a bizarre UX, now fixed.
Also should fix at least one capricious functional test :)
@durran durran force-pushed the derandomize-indexes-tab-order-flipping branch from 7ca0df0 to ee2ac56 Compare December 18, 2016 20:00
@durran
Copy link
Member

durran commented Dec 18, 2016

And I rebased against master as well. :)

@durran durran merged commit 5450934 into master Dec 18, 2016
@durran durran deleted the derandomize-indexes-tab-order-flipping branch December 18, 2016 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants