Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
entrywidth
andentrywidthmode
to legend #6202add
entrywidth
andentrywidthmode
to legend #6202Changes from all commits
3f473b1
6f0db3e
083d061
4c1a865
05b6d77
10d9d8e
08cb098
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I think we should provide a
dflt
for this attribute.However using zero looks confusing.
@alexcjohnson
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.
I actually think it's nice to use zero to mean "automatic - use the text width," if we document that.
Alternatively, we could have a third value for
entrywidthmode
, perhaps'auto'
, that means we use the text width. Then the logic would be:entrywidth
is provided,entrywidthmode
defaults to'pixel'
. If not, it defaults to'auto'
.entrywidthmode
is'auto'
we don't coerceentrywidth
.That feels unnecessarily cumbersome to me though. Only reason I can think of that would justify this is if we want to actually support a width of zero. Any reason someone would want that?
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.
If we want to use zero as the auto default the
min
namely for thepixel
mode should be a value greater or equal 1 not zero IMHO.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.
That would be confusing, wouldn't it? We'd document "Use 0 to size the entry based on the text width" but document a min of 1... yet if you explicitly set it to 0 it would work, because we'd throw out the 0 and replace it with the default 0.
Let's keep this simple, and consistent with
colorbar.len
andcolorbar.thickness
-min: 0
and nomax
regardless ofentrywidthmode
, the only difference being that0
has a special meaning we'll document. If it turns out that certain values (either very small or very large) cause problems worse than "it looks bad," we can address that during the drawing process when we know more than we do at the supplyDefaults step.