Skip to content

CSE Machine: Fix some UI & animations issues, and add tests #2936

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

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

CZX123
Copy link
Contributor

@CZX123 CZX123 commented Apr 14, 2024

Description

This PR fixes some UI & animations issues, and add some tests for the CSE machine's layout and animations.
Fixes #2921 fully by correctly calculating the width of the frame with a rune.
Fixes #2959.
Fixes #2716. The new result is shown below:

Changes

UI

  • Improved environment frame positioning, spacing is now more consistent.
  • Global frame default text of :::pre-declared names::: now should appear first, above all bindings
  • Arrows from arrays now always exit the array first before pointing to their targets.
    • If the arrow comes from the first unit of the array to itself, it will now form a circular loop
  • Changed height and width behavior for array and function values
    • _height and _width now only refer to the height and widths of the array boxes or closure circles only
    • ArrayValue now has a totalHeight and totalWidth property that stores the total height and width of nested values inside the array
    • FnValue and GlobalFnValue has a totalWidth property that also includes the tooltip width
  • Double quotes are used for strings in the stash now.

Animations

  • Fixed frame arrows not animating alongside the frame creation in FrameCreationAnimation
  • Fixed the animation function inside BaseAnimationComponent, so that the example below has the correct expected behavior:
    a.animateTo({ x: 100 });
    a.animateTo({ x: 200 }, { delay: 1 });

Testing

  • Add test cases for the CSE machine's layout for values
  • Add test cases for the CSE machine's AnimationComponent

@coveralls
Copy link

coveralls commented Apr 14, 2024

Pull Request Test Coverage Report for Build 15933732801

Details

  • 78 of 129 (60.47%) changed or added relevant lines in 17 files are covered.
  • 135 unchanged lines in 10 files lost coverage.
  • Overall coverage increased (+0.3%) to 44.988%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/features/cseMachine/components/ArrayUnit.tsx 1 2 50.0%
src/features/cseMachine/components/ControlItemComponent.tsx 0 1 0.0%
src/features/cseMachine/components/values/ArrayValue.tsx 16 19 84.21%
src/features/cseMachine/animationComponents/base/AnimationComponents.tsx 11 15 73.33%
src/features/cseMachine/tests/snapshots/CseMachineAnimation.tsx.snap 0 5 0.0%
src/features/cseMachine/CseMachineLayout.tsx 2 8 25.0%
src/features/cseMachine/animationComponents/FrameCreationAnimation.tsx 0 7 0.0%
src/features/cseMachine/CseMachineAnimation.tsx 1 11 9.09%
src/features/cseMachine/tests/snapshots/CseMachine.tsx.snap 0 14 0.0%
Files with Coverage Reduction New Missed Lines %
src/features/cseMachine/components/values/ArrayValue.tsx 2 91.46%
src/features/cseMachine/components/values/FnValue.tsx 4 77.12%
src/features/cseMachine/components/arrows/ArrowFromText.tsx 5 78.13%
src/features/cseMachine/components/Frame.tsx 7 86.56%
src/features/cseMachine/components/values/GlobalFnValue.tsx 7 74.22%
src/features/cseMachine/CseMachineUtils.ts 7 67.78%
src/features/cseMachine/animationComponents/base/AnimationComponents.tsx 10 77.85%
src/features/cseMachine/animationComponents/FrameCreationAnimation.tsx 22 20.2%
src/features/cseMachine/CseMachineLayout.tsx 31 67.12%
src/features/cseMachine/CseMachineAnimation.tsx 40 21.83%
Totals Coverage Status
Change from base Build 15933129669: 0.3%
Covered Lines: 10166
Relevant Lines: 21199

💛 - Coveralls

@CZX123 CZX123 self-assigned this Apr 16, 2024
@martin-henz martin-henz marked this pull request as ready for review June 11, 2025 01:21
@martin-henz martin-henz marked this pull request as draft June 11, 2025 01:21
@RichDom2185 RichDom2185 marked this pull request as ready for review June 27, 2025 18:51
@RichDom2185 RichDom2185 enabled auto-merge (squash) June 27, 2025 18:52
@RichDom2185 RichDom2185 requested review from sayomaki and removed request for alsonleej and chuckyang123 June 27, 2025 18:53
@RichDom2185
Copy link
Member

@sayomaki do you have time to review this? It's a pure FE change and the PR seems ready, I don't see a reason not to merge it if it's all good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
4 participants