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

Composite Pass-Through Properties + Rescale #1365

Closed
wants to merge 6 commits into from

Conversation

dahlbyk
Copy link
Contributor

@dahlbyk dahlbyk commented Feb 25, 2018

As proposed in #611 (comment):

Still in progress, but opening PR to check CI and get feedback. cc @gordonwoodhull

@dahlbyk dahlbyk force-pushed the composite-props branch 2 times, most recently from f68de65 to 181986e Compare February 26, 2018 05:01
@dahlbyk
Copy link
Contributor Author

dahlbyk commented Feb 26, 2018

There might be a larger bug here - my test is failing because resizing() on the children never goes to false

To fix this I've made coordinateGridMixin.resizing() a setter, too, so it can be overridden in compositeChart. Technically this is a change to the public API, but it's not currently documented.

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Feb 26, 2018

label and renderLabel updates should cascade to children

I don't have a specific need here, so I'm going to leave this change out of this PR. If someone else needs #1314 fixed, you'd likely follow a pattern similar to what's found here.

@gordonwoodhull any feedback?

@dahlbyk dahlbyk changed the title WIP: Composite Pass-Through Properties Composite Pass-Through Properties + Rescale Feb 26, 2018
@kum-deepak
Copy link
Collaborator

Many thanks for your work @dahlbyk, currently heavy work is going on branch 3.0. Will it be possible for you to rebase and target your PR onto 3.0?

@dahlbyk dahlbyk changed the base branch from master to 3.0 April 9, 2018 06:33
@dahlbyk
Copy link
Contributor Author

dahlbyk commented Apr 9, 2018

Will it be possible for you to rebase and target your PR onto 3.0?

Done. We'll see if tests still pass. 😀

I'll leave the 2.x version at da3c437...NewBoCo:composite-props-2.x in case anyone needs it.

@dahlbyk dahlbyk changed the base branch from 3.0 to develop April 20, 2018 21:05
@dahlbyk
Copy link
Contributor Author

dahlbyk commented Apr 25, 2019

Revisiting this (a year later!) as I'm finally upgrading from my patched v2 to v3. I've refactored a bit and included a fix for title/shareTitle as well.

@gordonwoodhull
Copy link
Contributor

Apologies for the long delay on this. I was reminded of this PR by a question on SO.

Nice, clean implementation and special thanks for including tests!

Released in 3.1.5 and will port to the es6 branch shortly.

Thanks @dahlbyk!!

@dahlbyk
Copy link
Contributor Author

dahlbyk commented Oct 4, 2019

Rebased v3.1.5 commits here: 1438f80...cdc1cda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants