-
Notifications
You must be signed in to change notification settings - Fork 193
Making Arrow filtering and Center Alignment toggles persistent across CSE updates #3887
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
Changes from all commits
22d7859
baebfb5
f02260a
321f22d
39a1ac0
2c3897c
44e1f30
95d00a6
26345cb
d6c6881
50f2b77
2ef3c48
99f8301
e00e5f2
25755a4
6906fe9
492dc91
7b32eb4
c7b6466
2cfc936
d4de2c8
a6eb238
547101e
b82a03d
7e2ea03
e37bb21
f6e6880
38f3060
f479f58
b47811a
1663799
e112dec
ed6cb06
f0e7938
86ae003
a5b7ae5
de48e60
d3879f2
0b5c3d8
c4c52af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -424,7 +424,12 @@ const Playground: React.FC<PlaygroundProps> = props => { | |||||||||||||
| const autorunButtonHandlers = useMemo(() => { | ||||||||||||||
| return { | ||||||||||||||
| handleEditorEval: () => { | ||||||||||||||
| const wasCenterAligned = CseMachine.getCenterAlignment(); | ||||||||||||||
| CseMachine.clearCachedLayouts(); | ||||||||||||||
| if (wasCenterAligned) { | ||||||||||||||
| CseMachine.toggleCenterAlignment(); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+427
to
+431
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manually toggling center alignment here is redundant and potentially ineffective.
Suggested change
References
|
||||||||||||||
|
|
||||||||||||||
| // reset stepper before evaluation | ||||||||||||||
| dispatch(WorkspaceActions.updateCurrentStep(-1, workspaceLocation)); | ||||||||||||||
| dispatch(WorkspaceActions.updateStepsTotal(0, workspaceLocation)); | ||||||||||||||
|
|
@@ -438,7 +443,7 @@ const Playground: React.FC<PlaygroundProps> = props => { | |||||||||||||
|
|
||||||||||||||
| dispatch(WorkspaceActions.evalEditor(workspaceLocation)); | ||||||||||||||
| CseMachine.setClearDeadFrames(false); | ||||||||||||||
| if (CseMachine.getCenterAlignment()) { | ||||||||||||||
| if (wasCenterAligned) { | ||||||||||||||
| CseMachine.toggleCenterAlignment(); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+446
to
448
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This toggle-back logic should be removed along with the initial toggle at line 430. As noted previously, the internal state management in References
|
||||||||||||||
| }, | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling
this.sliderRelease(preservedStep)insidecomponentDidUpdatewill trigger an infinite loop of evaluations.When an evaluation finishes,
needCseUpdatetransitions fromtruetofalse, triggering this block.sliderReleasethen callshandleEditorEval, which dispatchesevalEditor. InPlayground.tsx,evalEditorsetsupdateCse(the source forneedCseUpdate) back totrue. Once that evaluation completes,needCseUpdatebecomesfalseagain, and the cycle repeats.If the goal is to restore the step after a code change, this logic needs a guard to prevent re-triggering evaluation if it was already initiated by this restoration process, or the step reset logic in
Playground.tsxshould be adjusted instead.