Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Release 3.2.1 D3.V5 Fix #198

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Release 3.2.1 D3.V5 Fix #198

merged 1 commit into from
Feb 19, 2021

Conversation

msusol
Copy link
Collaborator

@msusol msusol commented Feb 18, 2021

  • Fix missing labels and other D3.V3 to D3.V5 issues.
  • Revert the indexing changes i.e. (startIndex - 1).
  • Removed some unused GLOBALs.

Fixing the missing Y-labels issue.
@msusol
Copy link
Collaborator Author

msusol commented Feb 18, 2021

@bmabey Are you available to merge this PULL request with the D3.V5 fix?

@bmabey bmabey merged commit 53758d1 into bmabey:master Feb 19, 2021
@bmabey
Copy link
Owner

bmabey commented Feb 19, 2021

Were the startIndex changes causing the problem? I thought they had properly defaulted it so it would not have changed anything.

bmabey pushed a commit that referenced this pull request Feb 19, 2021
Fixing the missing Y-labels issue.
@bmabey
Copy link
Owner

bmabey commented Feb 19, 2021

Regardless, thank you for doing this and making such a complete PR and release! I've merged it in and released it.

@msusol
Copy link
Collaborator Author

msusol commented Feb 19, 2021

I don't understand why the start Index change was made. I reverted all of those changes, and didn't see the need for the change. It wasn't what caused the main issue though.

There was a line of javascript chart.attr("class", "xaxis").call(xAxis); in ldavis.v3.0.0.js not related to D3.V5 upgrade that stopped working i guess. It has been there for awhile. I compared to original D3.V3 version.

There was also a missing D3.V5 function change (sqr root).

You're welcome!

@ed9w2in6
Copy link
Contributor

@msusol No, those changes are necessary for custom start_index to function.

I have created a new issue (#265) for this.

@ed9w2in6
Copy link
Contributor

@msusol
#266 should revert only the changes that involves startIndex.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants