feat: make scatter plot legends clickable #690

dv-usama-ansari · 2025-01-23T09:53:57Z

Closes https://github.com/datavisyn/reprovisyn/issues/2249

Developer Checklist (Definition of Done)

Issue

All acceptance criteria from the issue are met
Tested in latest Chrome/Firefox

UI/UX/Vis

Requires UI/UX/Vis review
- Reviewer(s) are notified @dvmoritzschoefl
- Review has occurred (link to notes)
- Feedback is included in this PR
- Reviewer(s) approve of concept and design

Code

Branch is up-to-date with the branch to be merged with, i.e., develop
Code is cleaned up and formatted
Unit tests are written (frontend/backend if applicable)
Integration tests are written (if applicable)

PR

Descriptive title for this pull request is provided (will be used for release notes later)
Reviewer and assignees are defined
Add type label (e.g., bug, feature) to this pull request
Add release label (e.g., release: minor) to this PR following semver
The PR is connected to the corresponding issue (via Closes #...)
Summary of changes is written

Summary of changes

Implement click event on the legend to toggle the points in the scatter plot

Screenshots

EDIT: As of 47071b9

2025-01-28.14-00-45.mp4

Additional notes for the reviewer(s)

@thinkh: What should be the behavior of the regression line when we filter out something using the legend items

wip: click changes the colors of points in an unexpected way

dvdanielamoitzi · 2025-01-23T12:11:49Z

As discussed with @dv-usama-ansari , this is already implemented in visyn_core, and ready to use 🙂

dv-usama-ansari · 2025-01-23T12:17:07Z

Implemented in 7d22667 and it works beautifully!

Thanks for the hint @dvdanielamoitzi!

dvmoritzschoefl · 2025-01-27T07:33:50Z

src/vis/scatter/ScatterVis.tsx

-          return <LegendItem key={c} color={colorMap(c)} label={c} onClick={() => onClick(c)} filtered={false} />;
-        })}
+        {categories.map((c) => (
+          <LegendItem key={c} color={colorMap(c)} label={c} onClick={() => onClick(c)} filtered={hiddenCategories.some((hc) => hc === c)} />


At this point, why dont you just pass the set to the component and use Set.has()?

dvmoritzschoefl · 2025-01-27T07:34:25Z

src/vis/scatter/ScatterVis.tsx

+          <Legend
+            categories={legendData.color.categories}
+            colorMap={legendData.color.mappingFunction}
+            hiddenCategories={[...hiddenCategoriesSet]}


Pass the set?

dvmoritzschoefl · 2025-01-27T07:45:24Z

src/vis/scatter/ScatterVis.tsx

  });

+  React.useEffect(() => {


This looks like a very flaky solution. In general I tried to remove all useEffects from the scatter plot, since they visually they introduce the problem of flickering (since they run 2-3 ms after the values changed) and there are just better alternatives.
Without thinking long about it, the solution that I had in mind is to pass the categories to useDataPreparation and just do the filtering the useMemo then this should not be necessary

dvmoritzschoefl · 2025-01-27T07:46:01Z

src/vis/scatter/ScatterVis.tsx

@@ -290,6 +313,13 @@ export function ScatterVis({
    internalLayoutRef,
  });

+  React.useEffect(() => {


This should not be necessary

dvmoritzschoefl · 2025-01-27T07:46:32Z

src/vis/scatter/ScatterVis.tsx

+              } else {
+                hiddenCategoriesSet.add(e);
+              }
+              calculateScatter([...hiddenCategoriesSet]);


Here we should just set the categories and useDataPreparation should react to that with useMemo

dvmoritzschoefl · 2025-01-27T07:46:59Z

src/vis/scatter/useDataPreparation.ts

-    ids.forEach((v, i) => {
-      idToIndex.set(v, i);
-    });
+  const calculateScatter = React.useCallback(


Move this code to the memo

dvmoritzschoefl · 2025-01-27T07:47:16Z

src/vis/scatter/useLayout.tsx

@@ -163,6 +164,7 @@ export function useLayout({
        height,
      };

+      setLayout(finalLayout);


Remove all those

dvmoritzschoefl · 2025-01-27T07:50:57Z

src/vis/scatter/useDataPreparation.ts

-      idToIndex,
-    };
-  }, [status, value]);
+      const finalScatter = {


In general:

The approach with useEffect and the callback broke multiple things. For instance the state is stale. When I switch between facet/scatter then the legend does not reset (e.g. its still hidden which is weird). Also the regression line switch does not work anymore as well as the scale switch. All these problems can be mitigated if the useMemo is left in place

There are the 4 cases for

scatter, splom, facet, subplots. They are split up because each case is slightly different. So for each of the 4 cases you need to move the filtering code to the useMemo case and use that data in the plotly component then it should work

…nd click

…o ua/reprovisyn-2249-make-scatter-plot-legend-clickable

dv-usama-ansari · 2025-01-28T13:02:56Z

As of 47071b9

2025-01-28.14-00-45.mp4

…nd-clickable

…yn_core into ua/reprovisyn-2249-make-scatter-plot-legend-clickable

dv-usama-ansari · 2025-01-30T11:45:11Z

As of e6cd58d

Fixed selection flickering:

2025-01-30.12-42-57.mp4

…nd-clickable

src/vis/scatter/useDataPreparation.ts

src/vis/scatter/ScatterVis.tsx

src/vis/scatter/useDataPreparation.ts

* chore: prepare next dev release * feat: make scatter plot legends clickable (#690) * feat(wip): make scatter plot legends clickable wip: click changes the colors of points in an unexpected way * fix: build * fix: make legend ui more intuitive to the user * fix: address PR feedback + rectify logic to filter out values on legend click * chore: add comments * basic principle * fix: selectedpoints computation * chore: improve tooltips * chore: cleanup --------- Co-authored-by: Moritz Heckmann <moritz.heckmann@datavisyn.io> * fix: removed use-deep-compare-effect, removed Awaited type (#716) - Removed **use-deep-compare-effect** from package.json, since we have our own version - Removed Awaited<T> type, since TS 4.5 has an inbuilt one - Moved 4 individual state properties to one common object, to remove flaky behavior (double renders with out-of-sync values) in case React will not batch the setStates * feat: add waitForClientConfig flag to allow skipping the wait for the clientConfig (#717) feat: add waitForClientConfig flag to skip waiting for the clientConfig * Prepare release version 16.1.0 --------- Co-authored-by: datavisyn-bot <> Co-authored-by: Usama Ansari <115616380+dv-usama-ansari@users.noreply.github.com> Co-authored-by: Moritz Heckmann <moritz.heckmann@datavisyn.io> Co-authored-by: Moritz Heckmann <77104411+dvmoritzschoefl@users.noreply.github.com> Co-authored-by: Michael Pühringer <51900829+puehringer@users.noreply.github.com>

feat(wip): make scatter plot legends clickable

42abfa9

wip: click changes the colors of points in an unexpected way

dv-usama-ansari self-assigned this Jan 23, 2025

dv-usama-ansari added release: minor type: feature labels Jan 23, 2025

fix: build

eb69d70

fix: make legend ui more intuitive to the user

7d22667

dvmoritzschoefl requested changes Jan 27, 2025

View reviewed changes

dv-usama-ansari added 3 commits January 28, 2025 13:59

fix: address PR feedback + rectify logic to filter out values on lege…

47071b9

…nd click

chore: add comments

fce2678

Merge branch 'develop' of https://github.com/datavisyn/visyn_core int…

5e67987

…o ua/reprovisyn-2249-make-scatter-plot-legend-clickable

dv-usama-ansari marked this pull request as ready for review January 28, 2025 13:39

dv-usama-ansari requested a review from a team as a code owner January 28, 2025 13:39

dvmoritzschoefl and others added 5 commits January 29, 2025 09:26

basic principle

19f2847

Merge branch 'develop' into ua/reprovisyn-2249-make-scatter-plot-lege…

5335a9a

…nd-clickable

Merge branch 'mh/filter_showcase' of https://github.com/datavisyn/vis…

ef91878

…yn_core into ua/reprovisyn-2249-make-scatter-plot-legend-clickable

fix: selectedpoints computation

e6cd58d

chore: improve tooltips

4de5324

Merge branch 'develop' into ua/reprovisyn-2249-make-scatter-plot-lege…

a52f3b6

…nd-clickable

dvmoritzschoefl requested changes Feb 3, 2025

View reviewed changes

src/vis/scatter/useDataPreparation.ts Outdated Show resolved Hide resolved

src/vis/scatter/ScatterVis.tsx Outdated Show resolved Hide resolved

src/vis/scatter/useDataPreparation.ts Outdated Show resolved Hide resolved

chore: cleanup

5e724b3

dvmoritzschoefl approved these changes Feb 3, 2025

View reviewed changes

thinkh merged commit e1eefbb into develop Feb 4, 2025
9 of 11 checks passed

thinkh deleted the ua/reprovisyn-2249-make-scatter-plot-legend-clickable branch February 4, 2025 08:20

github-actions bot mentioned this pull request Feb 11, 2025

Release 16.1.0 #721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make scatter plot legends clickable #690

feat: make scatter plot legends clickable #690

dv-usama-ansari commented Jan 23, 2025 •

edited

Loading

dvdanielamoitzi commented Jan 23, 2025

dv-usama-ansari commented Jan 23, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dvmoritzschoefl Jan 27, 2025

dv-usama-ansari commented Jan 28, 2025 •

edited

Loading

dv-usama-ansari commented Jan 30, 2025

feat: make scatter plot legends clickable #690

feat: make scatter plot legends clickable #690

Conversation

dv-usama-ansari commented Jan 23, 2025 • edited Loading

Developer Checklist (Definition of Done)

Summary of changes

Screenshots

Additional notes for the reviewer(s)

dvdanielamoitzi commented Jan 23, 2025

dv-usama-ansari commented Jan 23, 2025

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

Choose a reason for hiding this comment

dv-usama-ansari commented Jan 28, 2025 • edited Loading

dv-usama-ansari commented Jan 30, 2025

dv-usama-ansari commented Jan 23, 2025 •

edited

Loading

dv-usama-ansari commented Jan 28, 2025 •

edited

Loading