Skip to content
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

Added way for PolarAxis to pull fontsize attribute from Figure #4314

Merged
merged 15 commits into from
Sep 30, 2024

Conversation

NeunMonde
Copy link
Contributor

@NeunMonde NeunMonde commented Sep 4, 2024

Description

Fixes issue #3928.

Adjusted the "rticklabelsize" and "thetaticklabelsize" properties in the "types.jl" file to fall back to inheriting the fontsize from the Figure, if the ticklabelsize is not defined.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

Adjusted the "rticklabelsize" and "thetaticklabelsize" properties in the "types.jl" file to fall back to inheriting the fontsize from the Figure, if the ticklabelsize is not defined. This fixes issue MakieOrg#3928 (MakieOrg#3928).
Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Do you want to add a reference image test to make sure this works?

To do so, simply add another @reference_test block at the end of this file. https://github.com/MakieOrg/Makie.jl/blob/master/ReferenceTests/src/tests/figures_and_makielayout.jl

src/makielayout/types.jl Outdated Show resolved Hide resolved
asinghvi17

This comment was marked as duplicate.

@asinghvi17
Copy link
Member

Reference image changes are because the default font size in the polar axis is now inheriting from the default 14, not 16 as it was when the recipe was written. Given that this restores consistency with Axis, IMO this is for the better.

@asinghvi17
Copy link
Member

Could you also add a short entry in the CHANGELOG.md about the PR? Thanks!

…4314

yticklabelsize now propagates into rticklabelsize instead of thetaticklabelsize and xticklabelsize now propagates into thetaticklabelsize
> Actually I think this would be better as unit tests. There is already a refimg test for decorations that should confirm that changing r-/thetaticklabelsize has the desired effect. So here you just need to confirm that the PolarAxis is correctly inheriting values.
CHANGELOG.md Outdated Show resolved Hide resolved
test/PolarAxis.jl Outdated Show resolved Hide resolved
Copy link
Collaborator

@ffreyer ffreyer left a comment

Choose a reason for hiding this comment

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

Refimages just fail due to different default fontsize (inherited)

@ffreyer ffreyer merged commit 4f2971d into MakieOrg:master Sep 30, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

4 participants