search

CartoDB / carto-react

CARTO for React packages
https://docs.carto.com/react/
MIT License
38 stars 16 forks source link

fix(react-ui): Fix LegendProportion radius scale #877

Closed donmccurdy closed 4 months ago

donmccurdy commented 5 months ago

Description

Shortcut: Story 415171, URL redacted.

LegendProportion displays the legend for point scaling, and sometimes calculates the scale incorrectly. In the image below, note that the 'max' is smaller than the second number, and the 'min' is larger than the third.

The cause is a typo, we should be using max - min instead of max + min to calculate the interval over which the two middle steps are interpolated. With this bug, the steps were calculated correctly only when min was zero.

Additionally, the visual sizes of the circles in the legend panel are hard-coded to [3, 6, 9, 12], representing two evenly-spaced steps between the min and max. However, our implementation would have generated different steps given min=3, max=12 as input, because it divided the interval by 4, not 3.

before after
legend_range_bug Screenshot 2024-06-11 at 12 39 00 PM

Type of change

Acceptance

Please describe how to validate the feature or fix

  1. Link C4R into Builder

  2. Create a new map

  3. Add SQL query

    SELECT * FROM `carto-demo-data.demo_tables.cell_towers_worldwide` AS table
WHERE (table.lat - 36.744874) < 5 AND (table.lon - 3.195371) < 5;

  4. Set point layer to style radius by lat column

  5. Open legend, observe steps are correctly spaced

github-actions[bot] commented 5 months ago

Pull Request Test Coverage Report for Build 9469631381

Details

Totals Coverage Status
Change from base Build 9414264087: -0.009%
Covered Lines: 2804
Relevant Lines: 3627
💛 - Coveralls
github-actions[bot] commented 5 months ago

Pull Request Test Coverage Report for Build 9469631381

Details

Totals Coverage Status
Change from base Build 9414264087: -0.009%
Covered Lines: 2804
Relevant Lines: 3627
💛 - Coveralls
github-actions[bot] commented 5 months ago

Visit the preview URL for this PR (updated for commit 4916345):

https://cartodb-fb-storybook-react-dev--pr877-donmccurdy-fix-l-5w4wmit6.web.app

(expires Wed, 19 Jun 2024 14:32:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9469631381

Details

Totals Coverage Status
Change from base Build 9414264087: -0.009%
Covered Lines: 2804
Relevant Lines: 3627
💛 - Coveralls
github-actions[bot] commented 4 months ago

Pull Request Test Coverage Report for Build 9484450119

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

Totals Coverage Status
Change from base Build 9414264087: -0.004%
Covered Lines: 2805
Relevant Lines: 3628
💛 - Coveralls
github-actions[bot] commented 4 months ago

Pull Request Test Coverage Report for Build 9484450119

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

Totals Coverage Status
Change from base Build 9414264087: -0.004%
Covered Lines: 2805
Relevant Lines: 3628
💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9484450119

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

Totals Coverage Status
Change from base Build 9414264087: -0.004%
Covered Lines: 2805
Relevant Lines: 3628
💛 - Coveralls