frontend icon indicating copy to clipboard operation
frontend copied to clipboard

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

Open CZX123 opened this issue 1 year ago • 1 comments

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

  • [X] Improved environment frame positioning, spacing is now more consistent.
  • [X] Global frame default text of :::pre-declared names::: now should appear first, above all bindings
  • [X] 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
  • [X] 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
  • [X] Double quotes are used for strings in the stash now.

Animations

  • [X] Fixed frame arrows not animating alongside the frame creation in FrameCreationAnimation
  • [X] 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

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

CZX123 avatar Apr 14 '24 13:04 CZX123

Pull Request Test Coverage Report for Build 16860915421

Details

  • 151 of 197 (76.65%) changed or added relevant lines in 17 files are covered.
  • 12 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.2%) to 43.458%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/features/cseMachine/components/ControlItemComponent.tsx 1 2 50.0%
src/features/cseMachine/components/StashItemComponent.tsx 1 2 50.0%
src/features/cseMachine/components/Frame.tsx 39 43 90.7%
src/features/cseMachine/animationComponents/FrameCreationAnimation.tsx 1 8 12.5%
src/features/cseMachine/CseMachineLayout.tsx 12 28 42.86%
src/features/cseMachine/CseMachineAnimation.tsx 2 19 10.53%
<!-- Total: 151 197
Files with Coverage Reduction New Missed Lines %
src/features/cseMachine/animationComponents/FrameCreationAnimation.tsx 1 18.68%
src/features/cseMachine/CseMachineUtils.ts 1 71.65%
src/features/cseMachine/animationComponents/base/AnimationComponents.tsx 4 75.65%
src/features/cseMachine/components/arrows/ArrowFromText.tsx 6 81.82%
<!-- Total: 12
Totals Coverage Status
Change from base Build 16858234296: 0.2%
Covered Lines: 20627
Relevant Lines: 49670

💛 - Coveralls

coveralls avatar Apr 14 '24 13:04 coveralls

@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

RichDom2185 avatar Jun 27 '25 18:06 RichDom2185

Update: to be merged by 29/9/2025, to be ready for Unit 3 of CS1101S.

martin-henz avatar Aug 10 '25 09:08 martin-henz

Fixes #2959.

Still getting the issue with the sample program provided in the issue (+ other programs)

image image

I'll remove this from the issues closed by this PR. But feel free to close if I'm mistaken.

Fixes #2716.

Can't test right now due to issues with immer but I'll take your word for it.

RichDom2185 avatar Aug 10 '25 11:08 RichDom2185