Bugfix: first added locked figure settings accordion should be expanded on add

[nishasy]

Summary:

There was an issue where the first added locked figure settings would be collapsed, but all the settings are supposed to be expanded when added. This was happening because the initial expanded states array had one element in it when it was supposed to be empty.

Issue: https://khanacademy.atlassian.net/browse/LEMS-1992

Before

Screen.Recording.2024-05-16.at.11.48.49.AM.mov

After

Screen.Recording.2024-05-16.at.11.49.17.AM.mov

Test plan:

Storybook

• http://localhost:6006/?path=/story/perseuseditor-editorpage--demo
• Add an interactive graph widget
• Use a segment graph
• Click "Add locked element"
• Select any element
• The settings accordion should be expanded

[github-actions]

Size Change: +12 B (0%)

Total Size: 839 kB

Filename	Size	Change
packages/perseus-editor/dist/es/index.js	268 kB	+12 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/math-input/dist/es/strings.js	1.73 kB
packages/perseus-core/dist/es/index.js	908 B
packages/perseus-error/dist/es/index.js	877 B
packages/perseus-linter/dist/es/index.js	21.8 kB
packages/perseus/dist/es/index.js	403 kB
packages/perseus/dist/es/strings.js	3.22 kB
packages/pure-markdown/dist/es/index.js	3.68 kB
packages/simple-markdown/dist/es/index.js	12.4 kB

_{compressed-size-action}

[khan-actions-bot]

Gerald

Required Reviewers

• @Khan/perseus for changes to .changeset/tender-wombats-suffer.md, packages/perseus-editor/src/components/locked-figures-section.tsx

---

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.64%. Comparing base (e14a003) to head (0980aa7).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1280      +/-   ##
==========================================
+ Coverage   68.81%   70.64%   +1.83%     
==========================================
  Files         477      481       +4     
  Lines      101842   101909      +67     
  Branches     7213    10371    +3158     
==========================================
+ Hits        70082    71997    +1915     
+ Misses      31581    29912    -1669     
+ Partials      179        0     -179     

Files	Coverage Δ
...s-editor/src/components/locked-figures-section.tsx	100.00% <100.00%> (ø)

... and 147 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 e14a003...0980aa7. Read the comment docs.

[Myranae]

Looks good! I was able to follow the testing steps and saw the accordion starts expanded when locked figures are added. Added one question, but I don't think it's a blocker. Thanks! :)

[Myranae]

Question: Wouldn't the first part of this check be sufficient? Undefined would be falsy, so if figures is undefined, this equals the empty array, but also if it is zero it is falsy. So if there is no figures defined or if length is zero, it's falsy. So the only way props.figures?.length can be truthy is if it exists and is greater than 0.

[nishasy]

That's what I thought! My guess is that it saw the length is not null, and so it evaluated to true? which also = 1?

Would love if someone with better knowledge of JavaScript had some insight into this. (Maybe @benchristel?)

[jeremywiebe]

Wouldn't the first part of this check be sufficient?

This is subjective, but I'm not a fan of using bare numbers for truthy checks (if props.figures?.length). I find them tricky to reason about.

@Myranae is right about 0 being falsey though.

I think I'd re-arrange this slightly and avoid needing to know the details of what is and isn't falsy/truthy in JS.

    const collapsedStateArray =
        Array((props.figures ?? []).length)
            .fill(false);

[Myranae]

@jeremywiebe So if it's an empty array that you try to fill with false, it will just stay an empty array, right?

[jeremywiebe]

Correct. array.fill with one argument replaces all instances in the array with the provided argument.

[nishasy]

ah, that's much more elegant. thank you!

[github-actions]

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1280

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

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