Skip to content

Adapt range cell rendering based on the configured font size #1043

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

Merged
merged 3 commits into from
Jun 19, 2025

Conversation

lukasmasuch
Copy link
Collaborator

@lukasmasuch lukasmasuch commented Jun 18, 2025

Changes the range cell rendering to be adapted based on the configured font. Instead of a static 12px font size and 6px range height, we take 90% of the configure font size for the range label and use 50% of the font (em) height for the range visualization. This makes the range cell better adapted to the configured theme.

@lukasmasuch lukasmasuch changed the title [WIP] Allow rendering for range cell relative to root font size Allow rendering for range cell relative to root font size Jun 18, 2025
@lukasmasuch lukasmasuch marked this pull request as ready for review June 18, 2025 11:25
@lukasmasuch lukasmasuch changed the title Allow rendering for range cell relative to root font size Allow rendering for range cell relative to the configure font size Jun 18, 2025
@lukasmasuch lukasmasuch changed the title Allow rendering for range cell relative to the configure font size Adapt range cell rendering based on the configure font size Jun 18, 2025
@lukasmasuch lukasmasuch requested a review from Copilot June 18, 2025 11:29
@lukasmasuch lukasmasuch changed the title Adapt range cell rendering based on the configure font size Adapt range cell rendering based on the configured font size Jun 18, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the range cell rendering to dynamically size its label and bar based on the configured font size rather than using fixed pixel values.

  • Exposes getEmHeight from core to measure font heights.
  • Replaces static 12px font and 6px bar height with computed labelFont and rangeHeight based on theme.baseFontStyle.
  • Applies dynamic sizing in range-cell.tsx for both drawing and text placement.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/core/src/index.ts Exported getEmHeight to allow measuring font height.
packages/cells/src/cells/range-cell.tsx Replaced static sizing with computed labelFont, emHeight, and rangeHeight. Removed RANGE_HEIGHT constant.

Copy link
Collaborator

@BrianHung BrianHung left a comment

Choose a reason for hiding this comment

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

LGTM; refactors range cell to remove fixed constants.

@lukasmasuch lukasmasuch merged commit bdc787f into main Jun 19, 2025
8 checks passed
@lukasmasuch lukasmasuch deleted the fix/scaling-to-root-font-size branch June 19, 2025 08:35
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.

2 participants