Skip to content

fix(tick): respect bandSize in tick sizing#9827

Open
Adeyinka1 wants to merge 3 commits into
vega:mainfrom
Adeyinka1:fix/tick-bandSize-wiring
Open

fix(tick): respect bandSize in tick sizing#9827
Adeyinka1 wants to merge 3 commits into
vega:mainfrom
Adeyinka1:fix/tick-bandSize-wiring

Conversation

@Adeyinka1
Copy link
Copy Markdown

@Adeyinka1 Adeyinka1 commented Apr 12, 2026

Summary

This PR fixes an issue where bandSize is defined in TickConfig but has no effect on tick marks. close #9767

Problem

Currently:

  • size controls tick thickness correctly
  • bandSize is ignored for tick marks

This creates inconsistency between configuration and rendering behavior.

Solution

  • Added fallback logic in getBandSize() for tick marks
  • bandSize is used when size is not explicitly provided
  • size continues to take precedence

Changes

  • Updated src/channeldef.ts to include bandSize fallback for tick marks
  • Added tests to validate:
    • bandSize is respected when size is absent
    • size overrides bandSize when both are defined

Testing

  • Ran targeted tests:

@Adeyinka1 Adeyinka1 requested a review from a team as a code owner April 12, 2026 21:53
@Adeyinka1
Copy link
Copy Markdown
Author

Thanks for taking a look. I’ve implemented the bandSize fallback for tick sizing so that size still takes precedence, and I added targeted tests for both cases. Happy to adjust the approach if you’d prefer this handled differently.

@joelostblom
Copy link
Copy Markdown
Contributor

Thanks for making the PR! I think this could make sense technically to restore the effect of bandSize. However, I wonder if the more appropriate fix is not to change the documentation to advertise size instead and remove any relation of bandSize to tickConfig. As you discovered, it is already possible to use size for controlling the thick length and it seems odd to have two different knobs to control the same characteristics of the mark.

Let's wait to hear if someone else on the team with more historical context around why bandSize might be useful can chime in, so that we are not missing anything here.

@Adeyinka1
Copy link
Copy Markdown
Author

Thanks for the thoughtful feedback; that makes sense.

My initial assumption was that since bandSize is defined in TickConfig, it was intended to have an effect similar to other band-based marks. But I agree that having both size and bandSize control the same visual property could be confusing.

If the intent is for size to be the single source of control for tick thickness, then updating the documentation and removing bandSize from TickConfig might indeed be the cleaner approach.

Happy to adjust the PR in either direction depending on what’s preferred, whether that’s keeping the current fix or aligning with a documentation/cleanup change.

@yhoonkim
Copy link
Copy Markdown
Contributor

Thanks @Adeyinka1 for suggesting the fix! (thanks for pinging me @joelostblom )

I prefer changing the documentation (remove bandSize and promote to use size) than adding another knob to sync with bandSize.
(For me, it's natural to say tick's size is the length of it since it is just a line having thickness and length. It doesn't have "band" like a bar.)

@Adeyinka1
Copy link
Copy Markdown
Author

Thanks @yhoonkim for the clarification; that direction sounds good to me. I will update the PR to align with using size as the single control for ticks and shift this toward the documentation/config cleanup approach.

@Adeyinka1
Copy link
Copy Markdown
Author

I have updated the PR to follow the documentation/config cleanup direction:

  • removed the tick bandSize wiring change
  • removed the added bandSize tick tests
  • removed bandSize from TickConfig
  • updated the tick docs to promote size/thickness instead

I also reran the targeted tick tests locally after the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tick Config exports bandSize which seemingly has no effect; should it be size?

3 participants