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 duplicated logic for category selection in PieWidgetUI #332

Merged
merged 9 commits into from
Feb 15, 2022

Conversation

juandjara
Copy link
Contributor

@juandjara juandjara commented Feb 15, 2022

Description

This PR fixes the behaviour of PieWidgetUI clickEvent, removing duplicated logic that was re-appliying UI changes already applied in other useEffect calls. Before this changes, you couldn't recieve the onSelectedCategoriesChange event without affecting the UI of the chart. This change makes it so you can better control if the component is controlled or uncontrolled.

Type of change

  • Fix

Acceptance

Acceptance is described here in terms of the component API

  1. create a constant array with const categories = useRef([])
  2. Render a <PieWidgetUI />
  3. Pass as props selectedCategories={categories.current} and onSelectedCategoriesChange={(value) => console.log(value)}
  4. When clicked, the UI of the chart should not be modified (the clicked part should not be highlighted)

Basic checklist

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

@juandjara juandjara requested a review from Clebal February 15, 2022 10:08
@juandjara juandjara changed the title fix PieWidgetUI click event fix duplicated logic in PieWidgetUI click event Feb 15, 2022
@coveralls
Copy link
Collaborator

coveralls commented Feb 15, 2022

Pull Request Test Coverage Report for Build 1848313894

  • 2 of 14 (14.29%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 70.335%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/react-ui/src/widgets/PieWidgetUI.js 2 14 14.29%
Totals Coverage Status
Change from base Build 1848194146: -0.1%
Covered Lines: 1219
Relevant Lines: 1633

💛 - Coveralls

@juandjara juandjara changed the title fix duplicated logic in PieWidgetUI click event fix duplicated logic for category selection in PieWidgetUI Feb 15, 2022
Copy link
Contributor

@Clebal Clebal left a comment

Choose a reason for hiding this comment

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

🚀

@Clebal Clebal merged commit 60ded9b into master Feb 15, 2022
@Clebal Clebal deleted the bug/fix-pie-widget-ui-click-event branch February 15, 2022 17:11
juandjara added a commit that referenced this pull request Apr 5, 2022
This PR is meant to bring back a bug fix that was performed in PR #332 but overwritten in PR #341 (both from version 1.2.1-beta.8)
The bug fix consisted in fixing the category selection logic for the PieWidgetUI component to avoid using `labels` in the selected category comparison and use the categories from `data` instead
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.

3 participants