-
Notifications
You must be signed in to change notification settings - Fork 23
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
feature added: layer_symbol() supports "freetext", and add color, size options #107
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks a lot @frankyan, for the nice pull request. I think they are an important addition. I provide a few points of feedback on the pull request.
- Because (i) of modularity, and (ii) the automatic feature update I get automatically from branch names when I create a new release, and (iii) make it more manageable to review, I think this pull request should be separate, through 3 branches, in
- addition of size argument for all layer_* functions
- addition on color argument for all layer_* functions
- addition of text functionality
layer_symbol
is meant to be hidden, in favour of scope-specific layer_* functions (e.g. layer_text). In this way we modularise and keep arguments clean. So I suggest to implement alayer_text
function that would use layer_symbol that you have edited.
layer_text(Value < -1, text = "-" )
-
is there any restriction on the name
freetext
? If not I would replacefreetext
totext
everywhere in the code -
for the three functionalities and branches, size, color, and text we need (simple) unit tests
-
for the three functionalities and branches, size, color, and text we need to update the README.Rdm to include one example for each, and possibly the new layer_text function in the list at the beginning of the README
I am glad that you are interested in the additional functions. Thank you for your great guides and suggestions. I will follow your suggestions to modify the code and reorganize the branches. |
Hello @frankyan, any luck with the process? The layer size might have come in handy for me already :) |
Hello. I just returned back from Chinese New Year vacation. I will try to finish it this week. |
I hope you enjoyed. No stress of course. |
Any news by any chance? |
I have started a pull request of my own inspired by yours |
FYI, from my pull request, compared with yours, I missed two features
|
Add
freetext
,color
, andsize
to thelayer_symbol()
function. Then thelayer_symbol()
function is much more customizable. It can display values (char) in one column of the data tibble or any other freetext (char) instead of the predefined symbols. Besides, the color and size can be customized.Here are some examples.