Skip to content

fix(DesignerV2): Added forceSave on run button, fixed connection panel issue#9200

Merged
rllyy97 merged 3 commits into
mainfrom
rileyevans/fix/designer-v2-workflow-state-cleanup
May 20, 2026
Merged

fix(DesignerV2): Added forceSave on run button, fixed connection panel issue#9200
rllyy97 merged 3 commits into
mainfrom
rileyevans/fix/designer-v2-workflow-state-cleanup

Conversation

@rllyy97
Copy link
Copy Markdown
Contributor

@rllyy97 rllyy97 commented May 20, 2026

Commit Type

  • fix - Bug fix

Risk Level

  • Low - Minor changes, limited scope

What & Why

Added forceSave option to the FloatingRunButton component.
Fixed some standalone v2 issues.
Fixed some connection panel render issues.

Impact of Change

  • Users: Connections panel action list now renders icons inline and left-aligned instead of overlapping/centered
  • Developers: FloatingRunButton accepts a new forceSave prop; getConnectionConfiguration manifest param is now optional
  • System: Cleaner separation between designer identity (designerID) and workflow content state (WorkflowJson)

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Standalone designer (Standard + Consumption), connections panel with multiple connectors

Contributors

@rllyy97

Screenshots/Videos

N/A

rllyy97 added 2 commits May 20, 2026 13:54
…oatingRunButton

- Use WorkflowJson instead of Workflow type for standalone designer state
- Remove id spreading from workflow state updates (use separate designerID)
- Add forceSave prop to FloatingRunButton to allow saving even when not dirty
- Fix getConnectionConfiguration manifest parameter to be optional
- Simplify monitoring view workflow restoration
…lignment

- Remove position:absolute from action icon in NodeLinkButton (both v1 and v2)
- Left-align button content with justifyContent: flex-start
Copilot AI review requested due to automatic review settings May 20, 2026 20:05
@rllyy97 rllyy97 added the risk:low Low risk change with minimal impact label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(DesignerV2): Added forceSave on run button, fixed connection panel issue
  • Issue: None — title is concise and follows conventional commit scope/summary style. It clearly calls out the main behavioral change and the UI fix.
  • Recommendation: Keep as-is. If you want to be even more explicit you could mention the workflow identity change (e.g., "...and workflow state/ID cleanup"), but this is optional.

Commit Type

  • Properly selected (fix).
  • Note: Only one commit type is selected which is correct.

⚠️ Risk Level

  • Assessment: The PR is labeled risk:low in labels and the PR body selects "Low".
  • Issue: Based on the diff, this change impacts workflow identity handling (separating designerID from workflow id), removes auto-setting of workflow ids in several places, and changes save behavior by adding a forceSave prop to the FloatingRunButton. Those behaviors can affect saving/restore flows and consumers of the designer state — this increases the potential impact beyond a purely cosmetic fix.
  • Recommendation: Consider updating the declared risk to risk:medium to reflect that this touches workflow state/identity and run/save logic. If you keep risk:low, add explicit testing notes that verify no regressions in save/restore, run triggering, and any persisted workflow identifiers.

What & Why

  • Current: "Added forceSave option to the FloatingRunButton component. Fixed some standalone v2 issues. Fixed some connection panel render issues."
  • Issue: The What & Why is short but present. It would benefit from one or two sentences about why forceSave is needed and the motivation for changing workflow id handling (e.g., to keep designer identity stable and avoid unintended id collisions when restoring workflows).
  • Recommendation: Expand to include a brief rationale. Example: Added forceSave to allow runs when workflow hasn't been flagged dirty (needed for X scenario). Separated designerID from workflow.id to avoid reusing the same id across designer sessions which caused Y.

⚠️ Impact of Change

  • Issue: The PR body includes Users/Developers/System bullets which is good, but it should explicitly call out backward-compatibility considerations and any contract/API changes for consumers (e.g., FloatingRunButton now accepts forceSave prop — update any external callers or document if it's intended for internal use only).
  • Recommendation:
    • Users: Clarify UX/user visible changes (e.g., "Runs can be triggered even when the UI doesn't mark the workflow as dirty if forceSave is used").
    • Developers: Call out the exact public API change: FloatingRunButton adds optional forceSave?: boolean and getConnectionConfiguration second param is now optional — mention any callers that must be updated.
    • System: Note any storage/serialization implications from removing automatic id generation in saved/loaded workflows.

Test Plan

  • Assessment: The PR body currently has Unit tests and E2E tests unchecked and Manual testing checked.
  • Issue: The code diff actually includes new unit tests (nodeLinkButton.spec.tsx added in two places). The PR body is therefore inaccurate. Also there are no E2E tests added; if none are required, please state why.
  • Recommendation:
    • Update the Test Plan checkboxes to reflect reality: check Unit tests added/updated (these have been added for nodeLinkButton in both the designer and designer-v2 libs).
    • Ensure CI is green for the added tests and mention the test run(s) you used locally or in CI.
    • If E2E is intentionally omitted, add a short justification (e.g., UI layout tweak with unit coverage is sufficient; manual test steps included). If any manual regression test steps were executed for the save/restore workflow or FloatingRunButton forceSave behavior, add them here.
    • Provide exact file paths of new/changed tests in the Test Plan section to make verification easier (e.g., libs/designer-v2/.../nodeLinkButton.spec.tsx, libs/designer/.../nodeLinkButton.spec.tsx).

Contributors

  • Assessment: Contributors section includes @rllyy97.
  • Recommendation: If other teammates helped with reviews/ideas (PM/Design/QA), consider adding them for credit. Not required to pass.

⚠️ Screenshots/Videos

  • Assessment: Marked N/A. That is acceptable for these structural and small UI alignment fixes.
  • Recommendation: If you made visual changes that affect layout (action icons alignment), consider attaching a before/after screenshot to help reviewers validate the fix faster.

Summary Table

Section Status Recommendation
Title Keep as-is or optionally mention workflow id change
Commit Type No change needed
Risk Level ⚠️ Change label to risk:medium or justify low with extra tests/docs
What & Why Expand slightly with rationale for forceSave and id changes
Impact of Change ⚠️ Add backward-compat notes and caller impacts (FloatingRunButton/getConnectionConfiguration)
Test Plan Update checkboxes to indicate unit tests were added and provide CI/test run info
Contributors Optional: add other contributors if any
Screenshots/Videos ⚠️ Optional: add before/after screenshots for UI changes

Final Notes

  • Advised risk level: risk:medium (higher than the PR label risk:low). Reason: changes to workflow identity handling and save/run behavior can have broader effects than a purely visual or localized change. Please either change the PR label to risk:medium or expand the PR body with explicit test coverage and justification that the change is low risk.
  • Tests: There are new unit tests in the diff. Please update the Test Plan checkboxes to mark Unit tests added/updated and ensure CI is passing. Include the file paths of the tests in the Test Plan for reviewers:
    • libs/designer-v2/src/lib/ui/panel/connectionsPanel/allConnections/test/nodeLinkButton.spec.tsx
    • libs/designer/src/lib/ui/panel/connectionsPanel/allConnections/test/nodeLinkButton.spec.tsx
  • Suggested PR body edits (copy/paste friendly):
    • Under What & Why: "Why: forceSave allows runs to be triggered for workflows that are functionally unchanged but require a save prior to running in certain host scenarios (describe case). Separating designerID from workflow.id prevents accidental reuse of workflow ids across designer sessions and avoids collisions when restoring workflows from monitoring view."
    • Under Impact: Add: "Developers must update any external uses of FloatingRunButton that rely on the old save gating behavior. getConnectionConfiguration now has an optional _manifest param — callers that previously passed undefined should continue to work."
    • Under Test Plan: Check the unit-tests box, list the test files added, and add a short summary of manual regression steps run for the forceSave behavior and workflow id separation (e.g., "1) Open standalone designer, modify then undo changes, click run with forceSave true — confirm save executed; 2) Toggle monitoring view and back — confirm designerID persisted and workflow restores without id collision").

Please update the PR body with the above corrections and re-run CI. Once updated, I can re-review and confirm the label/test checklist matches the diff. Thank you for the clear commit message and the added unit tests — those made verification much easier.


Last updated: Wed, 20 May 2026 21:15:08 GMT

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a few Designer V2 UX and runtime issues: it introduces an opt-in forceSave capability for the V2 floating run button, improves connections panel icon/layout rendering, and refactors Standalone V2 to better separate “designer identity” from the workflow JSON content.

Changes:

  • Added optional forceSave prop to FloatingRunButton and used it to bypass the “skip save when not dirty” behavior.
  • Fixed connections panel action list/icon layout by removing absolute-positioned icons and normalizing icon sizing.
  • Updated Standalone Designer V2 workflow state handling (and made getConnectionConfiguration’s manifest arg optional).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libs/designer/src/lib/ui/panel/connectionsPanel/connectionsPanel.less Normalizes action icon sizing to avoid overlap/misalignment.
libs/designer/src/lib/ui/panel/connectionsPanel/allConnections/nodeLinkButton.tsx Removes absolute icon positioning; left-aligns button content.
libs/designer-v2/src/lib/ui/panel/connectionsPanel/allConnections/nodeLinkButton.tsx Same alignment fix for Designer V2 connections panel node links.
libs/designer-v2/src/lib/ui/FloatingRunButton/index.tsx Adds forceSave prop and integrates it into the save-skipping logic.
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesignerV2.tsx Separates designerID from workflow JSON; makes manifest param optional.
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesignerConsumptionV2.tsx Adjusts workflow updates from Copilot proposals (but currently risks dropping workflow.id).

Comment thread libs/designer-v2/src/lib/ui/FloatingRunButton/index.tsx
@rllyy97 rllyy97 enabled auto-merge (squash) May 20, 2026 20:18
@rllyy97 rllyy97 added risk:low Low risk change with minimal impact and removed risk:low Low risk change with minimal impact labels May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

📊 Coverage Check

🎉 All changed files have adequate test coverage!

@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 504

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 504

- Test rendering, icon display, left-alignment, dispatch behavior
- Cover both designer v1 and designer-v2 versions
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 500

@rllyy97 rllyy97 added risk:low Low risk change with minimal impact and removed risk:low Low risk change with minimal impact labels May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 500

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 500

@rllyy97 rllyy97 added risk:low Low risk change with minimal impact and removed risk:low Low risk change with minimal impact labels May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 500

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 500

@rllyy97 rllyy97 added risk:low Low risk change with minimal impact and removed risk:low Low risk change with minimal impact labels May 20, 2026
@rllyy97 rllyy97 merged commit 9df8d11 into main May 20, 2026
41 of 48 checks passed
@rllyy97 rllyy97 deleted the rileyevans/fix/designer-v2-workflow-state-cleanup branch May 20, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants