Comparative Category Widget #505

juandjara · 2022-10-25T11:00:23Z

Description

This PR implements part of the work proposed in RFC #479

This widget uses the component AnimatedNumber introduced in PR #504. Because of that, it includes the changes introduced in PR #504. Because of that, this PR should be merged only after PR #504 is merged

Changes added in this PR include:

A new widget called ComparativeCategoryWidgetUI in the widgets folder
Typescript typings for ComparativeCategoryWidgetUI and the exported enum ORDER_TYPES
JSDoc typings comments for ComparativeCategoryWidgetUI and the exported enum ORDER_TYPES
Prop-types validation for ComparativeCategoryWidgetUI and all the subcomponents included
A basic storybook story for ComparativeComparativeWidgetUI where all prop combinations can be tested and tweaked. A screenshot of this storybook is added as visual feedback.

Type of change

Feature

Acceptance

How to validate the feature:

Open the local storybook running yarn storybook:start in the react-ui folder.
Pick a set of properties to use for testing by tweaking the storybook configuration.
Use this set of properties to render a ComparativeCategoryWidgetUI in your sample project, by importing ComparativeCategoryWidgetUI from @carto/react-ui. You can use this branch of carto-react-template to skip this step if you want, as a test screen is already setup there. Bear in mind that if this feature is not published to npm you will have to link your local version of @carto/react-* packages with yarn link.
Check that the rendered version of the widget in your sample project looks the same as it looks in the storybook.

Basic checklist

Good PR name
Shortcut link
Changelog entry
Just one issue per PR
GitHub labels
Proper status & reviewers
Tests
Documentation

- typings - jsdoc - exports - storybook

coveralls · 2022-10-25T11:03:40Z

Pull Request Test Coverage Report for Build 3429857450

120 of 127 (94.49%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.7%) to 72.422%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/ComparativeCategoryWidgetUI.js	79	82	96.34%
packages/react-ui/src/widgets/comparative/ComparativeCategoryWidgetUI/CategoryItem.js	22	26	84.62%

Totals
Change from base Build 3425917787:	0.7%
Covered Lines:	1844
Relevant Lines:	2371

💛 - Coveralls

…ative-category-widget

VictorVelarde · 2022-10-28T14:39:50Z

packages/react-ui/src/widgets/ComparativeCategoryWidgetUI.js

+ * Enum for ComparativeCategoryWidgetUI order types. 'RANKING' orders the data by value and 'FIXED' keep the order present in the original data
+ * @enum {string}
+ */
+export const ORDER_TYPES = {


Can't we share same order types from CategoryWidget?

Sure, we could move the export const ORDER_TYPES to CategoryWidgetUI and use that instead of writing them to CategoryWidgetUI, so we can import them separately from ComparativeCategoryWidgetUI.

…comparative-category-widget

VictorVelarde · 2022-10-28T14:45:45Z

packages/react-ui/src/widgets/ComparativeCategoryWidgetUI.js

+  }
+}))(Tooltip);
+
+function CategoryItem({


I would strongly suggest splitting this long file, with different components, into different files, one per component. Also I think it would probably make sense to create a bit more of structure in folders:

widgets

CategoryWidgetUI

comparative

Category

ComparativeCategoryWidgetUI

CategoryItem

Skeleton...

Allright, ComparativeCategoryWidget is now split into multiple files and in the folder widgets/comparative

VictorVelarde · 2022-10-28T14:47:10Z

packages/react-ui/src/widgets/ComparativeFormulaWidgetUI.js

@@ -66,7 +67,7 @@ function ComparativeFormulaWidgetUI({
  }

  return (
-    <div>
+    <div className={classes.formulaChart}>


I've removed this class in the previous PR, as it has no content

oh sorry, I thought it was there for something. I will be removing it again

VictorVelarde · 2022-10-28T14:49:47Z

packages/react-ui/storybook/stories/widgetsUI/ComparativeCategoryWidgetUI.stories.js

@@ -0,0 +1,47 @@
+import React from 'react';


Stories file is obviously very useful for debugging, thx for doing it.

But something we're missing here, and in general in comparative is the unit testing side. Same as we have /tests_/widgets/CategoryWidgetUI.test.js we should have corresponding unit tests for each comparative widget.

That's gonna allow us to maintain it better in the future (or maybe to refactor it to share some pieces...). Would you please?
Would you please

Allright, I will be looking at existing test files and try to create similar ones for comparative widgets

And this one will be a task for tomorrow

juandjara · 2022-10-28T14:49:56Z

@VictorVelarde thanks a lot for your review!
I will be addressing these comments next monday

VictorVelarde · 2022-11-02T17:17:26Z

Hi @juandjara, I see you're advancing well. Ping us when this is ready for a 2nd review, thx!

…comparative-category-widget

juandjara · 2022-11-09T16:16:59Z

Hi @VictorVelarde! Tests for ComparativeCategoryWidgetUI and other changes are now ready. I've made sure to test almost the same behaviour as the CategoryWidgetUI is testing to ensure a consistent logic. Let me know if you have any more comments or any more feedback in order to finish this PR.

VictorVelarde · 2022-11-09T19:18:47Z

Thx a lot @juandjara, I think you have done a really nice job here ✨ . I'm gonna land it and make 1.5.0-alpha so you can properly review it in your project

VictorVelarde

🚀

juandjara added 8 commits October 24, 2022 14:04

feat(ui): implement ComparativeFormulaWidgetUI

ccd1eda

add storybook for ComparativeFormulaWidgetUI

2ef27fe

add changelog entry

f76ccb7

add JSDoc for ComparativeFormulaWidgetUI

faca867

add JSDoc for AnimatedNumber

c711fc3

export type not exported

831f9cf

implement comparative category widget UI

0cfe6fa

finish comparative category widget ui addons

67918b6

- typings - jsdoc - exports - storybook

juandjara changed the title ~~Feature/comparative category widget~~ Comparative Category Widget Oct 25, 2022

add changelog entry

99d15f1

juandjara requested a review from VictorVelarde October 25, 2022 11:07

juandjara added 5 commits October 26, 2022 14:01

implement custom animation hook to replace npm lib

67e294d

Merge branch 'feature/comparative-formula-widget' into feature/compar…

aa2738c

…ative-category-widget

fix animation options exported type

6a84ad4

Merge branch 'feature/comparative-formula-widget' into feature/compar…

eb4f8ff

…ative-category-widget

fix types and naming for comparative category widget

83b0adb

VictorVelarde reviewed Oct 28, 2022

View reviewed changes

juandjara added 2 commits October 28, 2022 16:42

Merge branch 'master' of github.com:CartoDB/carto-react into feature/…

0eec2fc

…comparative-category-widget

fix storybook for comparative category widget

207056e

VictorVelarde reviewed Oct 28, 2022

View reviewed changes

juandjara added 6 commits October 31, 2022 12:17

fix bug for empty comparative data

5036fcd

fix layout bug in bullets

fc2c1ab

reuse ORDER_TYPES from CategoryWidgetUI in ComparativeCategoryWidgetUI

7ab9e50

move comparative widgets to their own folder

3a1f9c0

split comparative category widgets in many files

7e09a14

remove unused class

74de180

juandjara added 5 commits November 3, 2022 13:32

fix layout for long overflowing category labels

e483417

fix compare value case for small reference value

5b9b327

fix edge case for empty color array

d68dac7

add tests for comparative category widget

d011829

Merge branch 'master' of github.com:CartoDB/carto-react into feature/…

256240b

…comparative-category-widget

VictorVelarde self-requested a review November 9, 2022 19:19

VictorVelarde approved these changes Nov 9, 2022

View reviewed changes

VictorVelarde merged commit b6b679f into master Nov 9, 2022

VictorVelarde deleted the feature/comparative-category-widget branch November 9, 2022 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comparative Category Widget #505

Comparative Category Widget #505

juandjara commented Oct 25, 2022 •

edited

Loading

coveralls commented Oct 25, 2022 •

edited

Loading

VictorVelarde Oct 28, 2022

juandjara Oct 28, 2022

juandjara Oct 31, 2022

VictorVelarde Oct 28, 2022

juandjara Oct 28, 2022

juandjara Oct 31, 2022

VictorVelarde Oct 28, 2022

juandjara Oct 28, 2022 •

edited

Loading

juandjara Oct 31, 2022

VictorVelarde Oct 28, 2022

juandjara Oct 28, 2022

juandjara Oct 31, 2022

juandjara commented Oct 28, 2022

VictorVelarde commented Nov 2, 2022

juandjara commented Nov 9, 2022

VictorVelarde commented Nov 9, 2022

VictorVelarde left a comment

Comparative Category Widget #505

Comparative Category Widget #505

Conversation

juandjara commented Oct 25, 2022 • edited Loading

Description

Type of change

Acceptance

Basic checklist

coveralls commented Oct 25, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3429857450

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juandjara Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juandjara commented Oct 28, 2022

VictorVelarde commented Nov 2, 2022

juandjara commented Nov 9, 2022

VictorVelarde commented Nov 9, 2022

VictorVelarde left a comment

Choose a reason for hiding this comment

juandjara commented Oct 25, 2022 •

edited

Loading

coveralls commented Oct 25, 2022 •

edited

Loading

juandjara Oct 28, 2022 •

edited

Loading