-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
move YAxis inside Axis #8253
move YAxis inside Axis #8253
Conversation
this.expandLastBucket = args.expandLastBucket == null ? true : args.expandLastBucket; | ||
this._attr = _.defaults(args._attr || {}); | ||
this.scale = null; | ||
this.domain = [args.yMin, args.yMax]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is domain only used by the yAxis? It would make sense for xAxis too right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its only used for yAxis ... the code in this PR is pretty bad as i only joined the 2 files to make it clear how i progressed. yAxis and xAxis have many functions that actually do same/similar thing but are named differently and sometimes work in different ways (like on one axis you are supposed to pass a scale to function which returns domain and on the other one you are supposed to pass in limits and then apply the return value on scale ... i wouldn't worry about all that in this PR ... its just to show steps of progress ... so putting all the functions from both files into one ... then we can clearly see in the diffs what have been changed from here on ...
Also, feel free to respond to any and all feedback with "I'll get there in a new pr" or something if you would rather push it in later. |
hmm ... not sure how i was testing, i was under impression that it works ok ... |
Well, I'm fine with it as an intermediate step :) |
2fast :) i was trying to push in some changes based on your comments + fix so this actually works as it should |
moving YAxis inside Axis