Add WB Tooltip to StyledMovablePoint #1192

mark-fitzgerald · 2024-04-18T21:23:12Z

Summary:

A tooltip for a movable point can be requested at the content creator's discretion. When this option is toggled "on" in the editor, a tooltip with the point's coordinates should appear whenever the user interacts with the point (either via mouse dragging or via keyboard movement).

Issue: LEMS-1860

Test plan:

Launch the developer testing app locally (yarn dev).
Navigate to localhost:5173 to view the various graphs.
Toggle one or more graphs to use Mafs (via the "Mafs Flags" dropdown menu).
Toggle the "Show Tooltips" switch.
Interact with a point via mouse to see the tooltip.
- Move your mouse pointer over one of the points in a Mafs graph and notice that a tooltip appears.
- Drag one of the points around on the graph and notice that the tooltip follows the point and updates the coordinate readout.
Interact with a point via keyboard to see the tooltip.
- Tab until one of the points has focus and notice that a tooltip appears.
- Move the point around by using the arrows on your keyboard and notice that the tooltip follows the point and updates the coordinate readout.

Affected Behavior

New Tooltip Functionality

Tooltip.on.Movable.Point.mov

Tooltip only shows when 'showTooltips' is true. Add tooltip toggle to gallery page for testing. Add tooltip functionality to segment graph.

… components.

…the tooltip will follow the point when it is moved. Add unit tests for including/excluding a tooltip based upon graph options.

… set and the user hovers or focuses on the point). Add "graphOptions" to the graph context to aid in passing graph settings to child components that need them for rendering. Adjust hairlines and tooltip colors to match movable point color. Add unit tests.

Make the tooltip background match the point color.

Resolve merge conflicts. Update unit tests that reference WB Tooltip to mock the component instead of inferring its existence.

github-actions · 2024-04-20T00:46:05Z

Size Change: +260 B (0%)

Total Size: 827 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	397 kB	+260 B (0%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.1 kB
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	80.5 kB
`packages/perseus-core/dist/es/index.js`	908 B
`packages/perseus-editor/dist/es/index.js`	267 kB
`packages/perseus-error/dist/es/index.js`	878 B
`packages/perseus-linter/dist/es/index.js`	21.8 kB
`packages/pure-markdown/dist/es/index.js`	3.68 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

Fix lint error.

mark-fitzgerald · 2024-04-20T00:53:39Z

dev/gallery.tsx

                        .map(([question, i]) => (
                            <QuestionRenderer
-                                key={i}
+                                key={`${i}${showTooltips ? "-with-tooltips" : ""}`}


Adding text to the key to force a re-render when toggling the "Show Tooltips" switch.

codecov · 2024-04-20T00:58:32Z

Codecov Report

Attention: Patch coverage is 88.88889% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 69.95%. Comparing base (890587e) to head (980d700).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1192      +/-   ##
==========================================
+ Coverage   68.56%   69.95%   +1.39%     
==========================================
  Files         464      467       +3     
  Lines       99360    99451      +91     
  Branches     7043    10694    +3651     
==========================================
+ Hits        68126    69572    +1446     
+ Misses      31119    29879    -1240     
+ Partials      115        0     -115

Files	Coverage Δ
packages/perseus/src/widgets/interactive-graph.tsx	`56.66% <100.00%> (+9.78%)`	⬆️
...seus/src/widgets/interactive-graphs/mafs-graph.tsx	`98.96% <100.00%> (+0.01%)`	⬆️
...ets/interactive-graphs/reducer/use-graph-config.ts	`100.00% <100.00%> (ø)`
...ractive-graphs/graphs/components/movable-point.tsx	`93.16% <87.87%> (+10.18%)`	⬆️

... and 121 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 890587e...980d700. Read the comment docs.

khan-actions-bot · 2024-04-20T01:02:48Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/dry-lemons-doubt.md, dev/gallery.tsx, packages/perseus/src/widgets/interactive-graph.tsx, packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx, packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx, packages/perseus/src/widgets/interactive-graphs/reducer/use-graph-config.ts, packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2024-04-22T22:46:02Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (980d700) and published it to npm. You
can install it using the tag PR1192.

Example:

yarn add @khanacademy/perseus@PR1192

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1192

nishasy

Very exciting! Could we consider giving the tooltip a fixed width if it's possible? To stop the size change from being so jarring while moving the point with the half steps adding more digits?

nishasy · 2024-04-22T22:47:09Z

dev/gallery.tsx

@@ -95,6 +95,9 @@ export function Gallery() {
    );

    const [isMobile, setIsMobile] = useState(params.get("mobile") === "true");
+    const [showTooltips, setShowTooltips] = useState(
+        params.get("tooltips") === "true",


Can this be shortened to useState(params.get("tooltips"))? or !!useState(params.get("tooltips"))?

I didn't see that "true" is a string. You can ignore my suggestion!

nishasy · 2024-04-22T22:49:54Z

packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx

    const hitboxRef = useRef<SVGCircleElement>(null);
    const {point, onMove, color = WBColor.blue} = props;

+    // WB Tooltip requires a color name for the background color.
+    // Since the color in props is a hex value, a reverse lookup is needed.


nishasy · 2024-04-22T22:53:28Z

packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx

I love these tests! 🎉

jeremywiebe

Overall I think this looks really good. I think the ability to color the movable point predates this PR so you can leave that for now, but I do know that currently Graphie doesn't allow you to change the color of the movables and so I think we want to do the same (ie. default to WB Blue and not allow customization).

jeremywiebe · 2024-04-23T01:02:52Z

.changeset/dry-lemons-doubt.md

+"@khanacademy/perseus": minor
+---
+
+Display tooltip for movable point when option is indicated


Our changelogs aren't critically important, but I think it'd be good to clarify here that we're talking about the interactive-graph widget.

jeremywiebe · 2024-04-23T01:35:22Z

packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx

@@ -20,10 +21,16 @@ type Props = {
 const hitboxSizePx = 48;

 export const StyledMovablePoint = (props: Props) => {
-    const {range, snapStep, markings} = useGraphConfig();
+    const {range, snapStep, markings, showTooltips} = useGraphConfig();
    const hitboxRef = useRef<SVGCircleElement>(null);
    const {point, onMove, color = WBColor.blue} = props;


Note that as far as I know, there are no plans to allow content authors to adjust the color of any of the movables. That is something that is strictly reserved for Locked Features.

So if it simplifies things, I think we can remove the ability to change the color of this StyledMovablePoint.

nishasy

You can ignore my suggestion about the width, since the current implementation matches the previous tooltip behavior. (Also just adding when we talked about online for the paper trail: it doesn't look that much better with a fixed/min width anyway, as the numbers are still kind of jumping back and forth.)

mark-fitzgerald · 2024-04-23T16:55:34Z

Very exciting! Could we consider giving the tooltip a fixed width if it's possible? To stop the size change from being so jarring while moving the point with the half steps adding more digits?

After looking into setting a minimum width for the tooltip bubble, the jarring aspect that occurs when moving is still there due to the snap-to-grid nature of the movement. Keeping the width consistent didn't produce the "smoothness" that we were hoping for. Leaving it without for now.

@jeresig

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/math-input@18.0.0 ### Major Changes - [#1168](#1168) [`a9c2308f9`](a9c2308) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n usage from perseus and perseus-editor packages. - [#1153](#1153) [`22709bd9b`](22709bd) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n from math-input, support multiple exports in rollup. ## @khanacademy/perseus@22.0.0 ### Major Changes - [#1168](#1168) [`a9c2308f9`](a9c2308) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n usage from perseus and perseus-editor packages. - [#1153](#1153) [`22709bd9b`](22709bd) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n from math-input, support multiple exports in rollup. ### Minor Changes - [#1186](#1186) [`d7fbe3e99`](d7fbe3e) Thanks [@nishasy](https://github.com/nishasy)! - Add UI within InteractiveGraphEditor to add/edit locked lines - [#1192](#1192) [`e86ed507e`](e86ed50) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - Display tooltip for movable point when option is indicated (Interactive Graph) - [#1195](#1195) [`9b5a9a40a`](9b5a9a4) Thanks [@nishasy](https://github.com/nishasy)! - Use MathJax colors for Interactive Graph locked figures - [#1195](#1195) [`9b5a9a40a`](9b5a9a4) Thanks [@nishasy](https://github.com/nishasy)! - View locked lines on Interactive Graph ### Patch Changes - [#1196](#1196) [`52b57c95d`](52b57c9) Thanks [@nishasy](https://github.com/nishasy)! - Rename ColorCircle to ColorSwatch - [#1198](#1198) [`890587ef1`](890587e) Thanks [@benchristel](https://github.com/benchristel)! - Internal: Add Storybook stories for visual regression testing of Mafs interactive graphs - Updated dependencies \[[`a9c2308f9`](a9c2308), [`22709bd9b`](22709bd)]: - @khanacademy/math-input@18.0.0 - @khanacademy/pure-markdown@0.3.2 ## @khanacademy/perseus-editor@6.0.0 ### Major Changes - [#1168](#1168) [`a9c2308f9`](a9c2308) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n usage from perseus and perseus-editor packages. - [#1153](#1153) [`22709bd9b`](22709bd) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n from math-input, support multiple exports in rollup. ### Minor Changes - [#1186](#1186) [`d7fbe3e99`](d7fbe3e) Thanks [@nishasy](https://github.com/nishasy)! - Add UI within InteractiveGraphEditor to add/edit locked lines - [#1195](#1195) [`9b5a9a40a`](9b5a9a4) Thanks [@nishasy](https://github.com/nishasy)! - Use MathJax colors for Interactive Graph locked figures - [#1195](#1195) [`9b5a9a40a`](9b5a9a4) Thanks [@nishasy](https://github.com/nishasy)! - View locked lines on Interactive Graph ### Patch Changes - [#1196](#1196) [`52b57c95d`](52b57c9) Thanks [@nishasy](https://github.com/nishasy)! - Rename ColorCircle to ColorSwatch - [#1201](#1201) [`125394c94`](125394c) Thanks [@jeanettehead](https://github.com/jeanettehead)! - Default a new radio widget to having four options - [#1200](#1200) [`d733aeaec`](d733aea) Thanks [@jeanettehead](https://github.com/jeanettehead)! - Prevent horizontal scrolling of radio widget options in the editor - Updated dependencies \[[`52b57c95d`](52b57c9), [`890587ef1`](890587e), [`d7fbe3e99`](d7fbe3e), [`e86ed507e`](e86ed50), [`a9c2308f9`](a9c2308), [`22709bd9b`](22709bd), [`9b5a9a40a`](9b5a9a4), [`9b5a9a40a`](9b5a9a4)]: - @khanacademy/perseus@22.0.0 - @khanacademy/math-input@18.0.0 ## @khanacademy/perseus-dev-ui@1.5.0 ### Minor Changes - [#1168](#1168) [`a9c2308f9`](a9c2308) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n usage from perseus and perseus-editor packages. ### Patch Changes - Updated dependencies \[[`a9c2308f9`](a9c2308), [`22709bd9b`](22709bd)]: - @khanacademy/math-input@18.0.0 - @khanacademy/pure-markdown@0.3.2 ## @khanacademy/pure-markdown@0.3.2 ## perseus-build-settings@0.4.0 ### Minor Changes - [#1168](#1168) [`a9c2308f9`](a9c2308) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n usage from perseus and perseus-editor packages. - [#1153](#1153) [`22709bd9b`](22709bd) Thanks [@jeresig](https://github.com/jeresig)! - Remove wonder-blocks-i18n from math-input, support multiple exports in rollup.

Mark Fitzgerald added 7 commits April 4, 2024 14:56

Add WB Tooltip to StyledMovablePoint.

4c90dc3

Tooltip only shows when 'showTooltips' is true. Add tooltip toggle to gallery page for testing. Add tooltip functionality to segment graph.

Refactor from prop-drilling to using context to pass graph options to…

0a85db1

… components.

Prettier formatting updates.

e0f5ec5

Merge main into branch.

170ffdd

Resolve linting issues.

86ecb48

Incorporate newly added 'autoUpdate' attribute in WB Tooltip so that …

d110508

…the tooltip will follow the point when it is moved. Add unit tests for including/excluding a tooltip based upon graph options.

mark-fitzgerald self-assigned this Apr 18, 2024

mark-fitzgerald requested a review from a team April 18, 2024 21:23

Mark Fitzgerald added 3 commits April 18, 2024 14:34

Remove testing code.

2431f4e

Fix update issue when toggling "Show Tooltips" switch in the gallery.

6b93e58

Make the tooltip background match the point color.

Merge main into branch.

45c23fb

Resolve merge conflicts. Update unit tests that reference WB Tooltip to mock the component instead of inferring its existence.

Mark Fitzgerald added 2 commits April 19, 2024 17:46

Remove changeset documents that have already been merged.

90bdf47

Remove unused variable ('graphOptions').

8f1dc9a

Fix lint error.

mark-fitzgerald commented Apr 20, 2024

View reviewed changes

mark-fitzgerald marked this pull request as ready for review April 20, 2024 01:02

Merge latest changes from main.

bb95fb9

nishasy reviewed Apr 22, 2024

View reviewed changes

Update snapshots with correct "data-testid" refrencing.

426154a

jeremywiebe approved these changes Apr 23, 2024

View reviewed changes

Mark Fitzgerald added 2 commits April 23, 2024 09:24

Clarify changelog entry.

696f27c

Merge branch 'main' into LEMS-1860-tooltips-in-mafs

980d700

nishasy approved these changes Apr 23, 2024

View reviewed changes

mark-fitzgerald merged commit e86ed50 into main Apr 23, 2024
14 checks passed

mark-fitzgerald deleted the LEMS-1860-tooltips-in-mafs branch April 23, 2024 16:56

khan-actions-bot mentioned this pull request Apr 23, 2024

Version Packages #1203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WB Tooltip to StyledMovablePoint #1192

Add WB Tooltip to StyledMovablePoint #1192

mark-fitzgerald commented Apr 18, 2024 •

edited

github-actions bot commented Apr 20, 2024 •

edited

mark-fitzgerald Apr 20, 2024

codecov bot commented Apr 20, 2024 •

edited

khan-actions-bot commented Apr 20, 2024 •

edited

github-actions bot commented Apr 22, 2024 •

edited

nishasy left a comment

nishasy Apr 22, 2024

nishasy Apr 22, 2024

nishasy Apr 22, 2024

nishasy Apr 22, 2024

jeremywiebe left a comment

jeremywiebe Apr 23, 2024

jeremywiebe Apr 23, 2024

nishasy left a comment

mark-fitzgerald commented Apr 23, 2024

Add WB Tooltip to StyledMovablePoint #1192

Add WB Tooltip to StyledMovablePoint #1192

Conversation

mark-fitzgerald commented Apr 18, 2024 • edited

Summary:

Test plan:

Affected Behavior

New Tooltip Functionality

github-actions bot commented Apr 20, 2024 • edited

mark-fitzgerald Apr 20, 2024

Choose a reason for hiding this comment

codecov bot commented Apr 20, 2024 • edited

Codecov Report

khan-actions-bot commented Apr 20, 2024 • edited

Gerald

github-actions bot commented Apr 22, 2024 • edited

npm Snapshot: Published

nishasy left a comment

Choose a reason for hiding this comment

nishasy Apr 22, 2024

Choose a reason for hiding this comment

nishasy Apr 22, 2024

Choose a reason for hiding this comment

nishasy Apr 22, 2024

Choose a reason for hiding this comment

nishasy Apr 22, 2024

Choose a reason for hiding this comment

jeremywiebe left a comment

Choose a reason for hiding this comment

jeremywiebe Apr 23, 2024

Choose a reason for hiding this comment

jeremywiebe Apr 23, 2024

Choose a reason for hiding this comment

nishasy left a comment

Choose a reason for hiding this comment

mark-fitzgerald commented Apr 23, 2024

mark-fitzgerald commented Apr 18, 2024 •

edited

github-actions bot commented Apr 20, 2024 •

edited

codecov bot commented Apr 20, 2024 •

edited

khan-actions-bot commented Apr 20, 2024 •

edited

github-actions bot commented Apr 22, 2024 •

edited