add new params to control tick labels#43
Conversation
|
These new params use a similar naming convention to the label angles. |
alex-hhh
left a comment
There was a problem hiding this comment.
There seem to be other places in the code that are affected by whether labels are drawn or not and these need to be updated as well.
I think the name of the parameters are OK and they can be documented as such in the scribble document
| (: get-x-far-tick-label-params (-> (Listof Label-Params))) | ||
| (define (get-x-far-tick-label-params) | ||
| (if (and (plot-x-far-axis?) draw-x-far-tick-labels?) | ||
| (if (or (and (plot-x-far-axis?) draw-x-far-tick-labels?) (and (plot-x-far-axis?) (plot-x-far-tick-labels?))) |
There was a problem hiding this comment.
I think that draw-x-far-tick-labels? should be updated to include plot-x-far-tick-labels? in the test and this line should remain unchanged. There are other methods which might be affected by the position of the plot labels, for example max-x-far-tick-label-height and get-x-far-tick-label-params.
| (: get-x-tick-label-params (-> (Listof Label-Params))) | ||
| (define (get-x-tick-label-params) | ||
| (if (plot-x-axis?) | ||
| (if (and (plot-x-axis?) (plot-x-tick-labels?)) |
There was a problem hiding this comment.
We might need a new variable, draw-x-tick-labels?, similar to draw-x-far-tick-labels? as the code seems to assume that they are always drawn (see max-x-tick-label-height, get-x-tick-label-params), this line should be
(if (and (plot-x-axis?) draw-x-tick-labels?) ...)... and the other places updated too.
|
Thanks for the ping. I had tried making a test case that would show the problems Alex is worried about, but no luck so far. Hoping to try again tonight/tomorrow. Can you add documentation for the new parameters? |
|
With the original sample code given in this pull request, try zooming the plot.... The labels will go back to the left and will stay there... |
|
Here is what it looks like. The problem is that plot parameters are taken from one plot-area to construct another one, and the information of where the labels is drawn is lost, since the relevant methods |
|
Thanks for providing examples with buggy behavior, @alex-hhh . I'll try to fix this up shortly. |
|
The code has been updated based on feedback. There are a few things to note:
Also, without the hardcoding, it will keep the far tick labels and not show near tick labels when you view the plot in a new window. I would like to know if you attempt to reproduce that.
|
|
Hi @evdubs , the parameters don't maintain their values across different threads, and the Below is the diff that fixes the problem: I added the parameters to the The rest of the changes look good to me. plot-lib/plot/private/common/parameter-groups.rkt | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/plot-lib/plot/private/common/parameter-groups.rkt b/plot-lib/plot/private/common/parameter-groups.rkt
index 3c2ffad..7b965e5 100644
--- a/plot-lib/plot/private/common/parameter-groups.rkt
+++ b/plot-lib/plot/private/common/parameter-groups.rkt
@@ -19,7 +19,14 @@
plot-y-tick-label-anchor
plot-y-tick-label-angle
plot-y-far-tick-label-anchor
- plot-y-far-tick-label-angle))
+ plot-y-far-tick-label-angle
+ plot-x-tick-labels?
+ plot-y-tick-labels?
+ plot-z-tick-labels?
+ plot-x-far-tick-labels?
+ plot-y-far-tick-labels?
+ plot-z-far-tick-labels?
+ ))
(define-parameter-group plot-appearance
(plot-width
@@ -89,7 +96,7 @@
Anchor
Nonnegative-Real
(List Boolean Boolean Boolean Boolean Boolean Boolean)
- (List Anchor Real Anchor Real Anchor Real Anchor Real)
+ (List Anchor Real Anchor Real Anchor Real Anchor Real Boolean Boolean Boolean Boolean Boolean Boolean)
Boolean
Boolean)
(List Positive-Integer Real Real Nonnegative-Real Boolean Boolean) |
|
I updated |
|
@bennn have any thoughts? I think this is ready for review now. |
|
Looks good! (Sorry for taking so long.) |
add new params to control tick labels (racket#43)
This uses a different approach to showing ticks but hiding labels using feedback from @bennn and @alex-hhh on PR #41
This was tested with the following code:
This will produce:
I suppose some documentation will be nice here even though this seems much more straight forward.