Skip to content

chore: rm Scheduled Execution Callback from Flow Canvas#1916

Draft
camielvs wants to merge 1 commit intomasterfrom
03-09-chore_rm_scheduled_execution_callback_from_flow_canvas
Draft

chore: rm Scheduled Execution Callback from Flow Canvas#1916
camielvs wants to merge 1 commit intomasterfrom
03-09-chore_rm_scheduled_execution_callback_from_flow_canvas

Conversation

@camielvs
Copy link
Collaborator

@camielvs camielvs commented Mar 9, 2026

Description

I may be mistaken but I think this code can be removed as the fit-to-view looks like it's successfully being taken care of elsewhere. Needs manual testing (as I don't remember why it was originally added). Hence, I've made it a separate PR.

Follow-up:

I have been exploring the implications of this PR. The removed code is currently delaying the canvas fit-view operation until after it has loaded, but before it renders. This makes the canvas load with the view centered on the content. Removal of this code causes the fit operation to happen after rendering, resulting in an animation. See the difference below. Do we want/need this? How do we feel about it?

Before (existing behaviour)

fit-view-no-animation.mov (uploaded via Graphite)

After (new behaviour)

fit-view-animated.mov (uploaded via Graphite)

Related Issue and Pull requests

Type of Change

  • Cleanup/Refactor

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Screenshots (if applicable)

Test Instructions

Additional Comments

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

🎩 To tophat this PR:

You can add the following URL parameter to your browser to tophat this PR:

`?tophat_location=03-09-chore_rm_scheduled_execution_callback_from_flow_canvas/372a6a3`

Copy link
Collaborator Author

camielvs commented Mar 9, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@camielvs camielvs mentioned this pull request Mar 9, 2026
3 tasks
@camielvs camielvs force-pushed the 03-09-chore_rm_scheduled_execution_callback_from_flow_canvas branch 2 times, most recently from ae66f01 to 1c8ea15 Compare March 9, 2026 19:15
@camielvs camielvs force-pushed the 03-06-feat_link_to_node_on_canvas branch from 6c1e47d to 571a7c4 Compare March 9, 2026 19:15
@camielvs camielvs marked this pull request as ready for review March 9, 2026 19:21
@camielvs camielvs requested a review from a team as a code owner March 9, 2026 19:21
@camielvs camielvs changed the base branch from 03-06-feat_link_to_node_on_canvas to graphite-base/1916 March 11, 2026 19:13
@camielvs camielvs force-pushed the graphite-base/1916 branch from 571a7c4 to 75dd6b7 Compare March 11, 2026 19:16
@camielvs camielvs force-pushed the 03-09-chore_rm_scheduled_execution_callback_from_flow_canvas branch from 1c8ea15 to 1e00c3d Compare March 11, 2026 19:16
@graphite-app graphite-app bot changed the base branch from graphite-base/1916 to master March 11, 2026 19:16
@camielvs camielvs force-pushed the 03-09-chore_rm_scheduled_execution_callback_from_flow_canvas branch from 1e00c3d to 372a6a3 Compare March 11, 2026 19:17
Copy link
Collaborator

I see e2e tests may become flaky because of the animation, could you take a look before merging?

Copy link
Collaborator Author

I think I'm going to try take a bit of time to reevaluate this PR and figure out if there a better way to deal with fitView on page load whilst still avoiding an animation and duplicated code.

@camielvs camielvs marked this pull request as draft March 12, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants