Skip to content

Fixed issue - column order changes on hover - #6718

Merged
archmoj merged 7 commits into
plotly:masterfrom
bhavinencore:feature/fix6699-column-order-changes-on-hover
Oct 10, 2023
Merged

archmoj merged 7 commits into
plotly:masterfrom
bhavinencore:feature/fix6699-column-order-changes-on-hover

Conversation

@bhavinencore

Copy link
Copy Markdown
Contributor

Thanks for your interest in plotly.js!

Translations:

  • Please @ mention a few other speakers of this language who can help review your translations.
  • If you've omitted any keys from dist/translation_keys.txt - which means they will fall back on the US English text - just make a short comment about why in the PR description: the English text works fine in your language, or you would like someone else to help translating those, or whatever the reason.
  • You should only update files in lib/locales/, not those in dist/

Features, Bug fixes, and others:

Before opening a pull request, developer should:

  1. make sure they are not on the master branch of their fork as using master for a pull request would make it difficult to fetch upstream changes.
  2. fetch latest changes from upstream/master into your fork i.e. origin/master then pull origin/master from you local master.
  3. then git rebase master their local dev branch off the latest master which should be sync with upstream/master at this time.
  4. make sure to not git add the dist/ folder (the dist/ is updated only on version bumps).
  5. make sure to commit changes to the package-lock.json file (if any new dependency required).
  6. provide a title and write an overview of what the PR attempts to do with a link to the issue they are trying to address.
  7. select the Allow edits from maintainers option (see this article for more details).

After opening a pull request, developer:

  • should create a new small markdown log file using the PR number e.g. 1010_fix.md or 1010_add.md inside draftlogs folder as described in this README, commit it and push.
  • should not force push (i.e. git push -f) to remote branches associated with opened pull requests. Force pushes make it hard for maintainers to keep track of updates. Therefore, if required, please fetch upstream/master and "merge" with master instead of "rebase".

Comment thread src/plots/plots.js Outdated
categoriesAggregatedValue.sort(function(a, b) {
return a[1] - b[1];
});
categoriesAggregatedValue.sort((a,b) => sortValues(a, b, order));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting, so the issue was a mismatch between ax._categoriesAggregatedValue and ax._initialCategories?

Performance will be better if we make two functions, sortAscending and sortDescending, and pass one or the other to sort rather than a wrapper function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the issue was a mismatch between ax._categoriesAggregatedValue and ax._initialCategories.

I have created two separate functions sortAscending and sortDescending.

@uraani

uraani commented Sep 18, 2023

Copy link
Copy Markdown

Could this be merged?

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Thanks for the nudge @uraani - This looks good to me, @archmoj any comments?

@uraani

uraani commented Oct 9, 2023

Copy link
Copy Markdown

Nudging again 😄

@archmoj archmoj added bug something broken status: reviewable community community contribution labels Oct 10, 2023
@archmoj

archmoj commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken community community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants