-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add :col_max_length
setting
#51
base: master
Are you sure you want to change the base?
Conversation
Allows for a maximum column width to be specified for charts of type `:column` or `:stack`.
Hello @grantneufeld and thanks for stopping by and for using Squid! Can you help me understand what is a use-case for this new setting? Thanks! |
However, before you accept this pull request, I’m thinking now that there may be a better way to define the column max width setting — especially if Squid is to ever offer other options for modifying columns (gradient fills, picture fills, borders, shadows, etc.). Perhaps something like a So, instead of |
I see. Thanks for your reply! I never thought of having options that are specific to a column. If you are thinking of moving in that direction, maybe you can try different attempts in your local branch / fork until you find one that's completely satisfying for you. At that point, you might update your PR and it will be easier for me to get a full overview of the changes required to the API compared with the advantages of the new settings. Thanks again! |
Will do. |
I’ve completely rethought the approach I took in this pull request. Instead of adding the very limited and specific So far, I’ve only implemented custom “renderer” support for plain (not stacked) columns, and the little boxes in the legend. You can see the code I’ve put together for this (including working I’ve tried to, through the examples, provide detailed documentation for using the renderer objects. Before I try another pull request here, please let me know what you think of this whole idea, and what improvements/changes could make it more suited for inclusion in Squid. Thanks! |
@grantneufeld wow, you went all the way with your research! Good job!! 👏 🎀 If I understand correctly, you are thinking of splitting the work of Squid into a base part (that calculates where the points will be plotted) and a rendered (which plots those point as graphical elements). I think the idea is good. In my code, I call "plotter" what you call "renderer". As you write in your branch, this opens some questions regarding configuration, objects, code complexity, which makes me think we would still need some work on top of your branch to have this all sketched out. For the current version, I don't have the time to delve into a re-organization of the API to make room for that. I'm happy to follow your progress in your local branch and see where that brings you, and eventually consider this new structure for the next major version of the gem. Thanks again 🎀 |
Yes, splitting the drawing/plotting/rendering behaviour from the position/scale/calculating behaviour should make customizing/changing the chart formatting quite flexible and easier to extend. I’m not sure when I’ll get another block of time to continue this work — I hope soon, but can’t make a specific commitment, as my workload is a bit full. |
I did a little more work on this, implementing support for customizing the axis labels/ticks: Slowly getting there… 😊 |
Allows for a maximum column width to be specified for charts of type
:column
or:stack
.