Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Consolidated code for drawing x-axis label #2167

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

wimrijnders
Copy link
Contributor

The drawing of the axis labels occurs in two places and are chunks of non-trivial code. Hence I think it is a good idea to consolidate these.

This PR contains a new single method for drawing labels for the x-axis. If you can agree with this, I will do the y and z axis labels in a similar manner. Awaiting your response.

Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

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

I like it! Keeping things DRY!
I'd like @mojoaxel just to take a look at it again just to make sure he's fine with it too.

@wimrijnders
Copy link
Contributor Author

OK, fine with me.
If you're both good with it, perhaps I can add the Y and Z label handling to this PR as well.

@yotamberk
Copy link
Contributor

That sounds good to me

@mojoaxel
Copy link
Member

@wimrijnders Looks good. Feel free to add more changes concerning the axis labels.

@wimrijnders
Copy link
Contributor Author

Ok. I'm missing my upward thumb emoticon here - (y) in skype.

Will do it this afternoon, have some other tasks now.

@yotamberk yotamberk merged commit db89162 into almende:develop Oct 18, 2016
@wimrijnders
Copy link
Contributor Author

Oh, merged already. Never mind, I'll make a new PR for the Y and Z labels.

@mojoaxel
Copy link
Member

mojoaxel commented Oct 18, 2016

Ok. I'm missing my upward thumb emoticon here - (y) in skype.

start with a ":"

http://www.webpagefx.com/tools/emoji-cheat-sheet/

@wimrijnders
Copy link
Contributor Author

👍 Ha! Thank you. Very important, this.

@wimrijnders wimrijnders deleted the PR8 branch October 18, 2016 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants