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

Fix - setting width/height with CSS units for metrics #2309

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

philliparaujo
Copy link

@philliparaujo philliparaujo commented Dec 9, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

The Metric component was not responding to size style values with units such as width='100px' or height='100%' because it uses an external Plot library that only accepts numerical sizes. This change adds a function that converts any size dimensions into a number of pixels, which can then be interpreted by the Plot properly.

Related Tickets & Documents

How to reproduce the issue

The file doc/gui/examples/controls/css_bug.py has example components and styles that previously did not respond to variable width/height Metric components. Running this file now should show the Metric component properly working.

Checklist

We encourage you to keep the code coverage percentage at 80% and above.

  • Does this solution meet the acceptance criteria of the related issue?
  • Is the related issue checklist completed?
  • Does this PR adds unit tests for the developed code? If not, why?
  • End-to-End tests have been added or updated?
  • Was the documentation updated, or a dedicated issue for documentation created? (If applicable)
  • Is the release notes updated? (If applicable)

philliparaujo and others added 5 commits December 8, 2024 20:08
- Add test for default metric behavior
- Document bug where numeric width/height values default to 100%
- Document bug where px unit width/height values are ignored
- Tests verify current behavior for future fix verification
I added several unit tests for the NormalizeSize function to ensure that our code meets coverage requirements and to validate the correctness of the function
@philliparaujo philliparaujo marked this pull request as ready for review December 9, 2024 01:34
@FlorianJacta FlorianJacta added the 🖰 GUI Related to GUI label Dec 9, 2024
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

What's been used in chart.tsx should be used here (share code)

Copy link
Contributor

This PR has been labelled as "🥶Waiting for contributor" because it has been inactive for more than 14 days. If you would like to continue working on this PR, then please add new commit or another comment, otherwise this PR will be closed in 14 days. For more information please refer to the contributing guidelines.

@github-actions github-actions bot added the 🥶Waiting for contributor Issues or PRs waiting for a long time label Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖰 GUI Related to GUI 🥶Waiting for contributor Issues or PRs waiting for a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 BUG] Setting width/height with CSS units not working for metrics
6 participants