Skip to content

refactor(frontend): remove dead breakpoint popup positioning - #9113

Merged
kunwp1 merged 2 commits into
apache:mainfrom
aglinxinyuan:refactor/fe-breakpoint-input-dead-positioning
Oct 25, 2026
Merged

kunwp1 merged 2 commits into
apache:mainfrom
aglinxinyuan:refactor/fe-breakpoint-input-dead-positioning

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Removes BreakpointConditionInputComponent.left() and top(), two positioning methods that nothing in production calls. −66 lines. There is no behaviour change.

The popup is positioned by ngOnChanges, which computes topPosition and leftPosition for the template's [ngStyle]="{ top: topPosition, left: leftPosition }" binding. The template never calls left() or top(). The component's one host, code-debugger, binds only its inputs and output, and the two parent ViewChild fields that reference the component are never read. Every import is still used.

The spec loses the five cases that only exercised the two methods. Its coverage of the surviving positioning (ngOnChanges, topPosition, leftPosition) stays.

History

Introduced by #2953 (2024-10-25), "Add Udon breakpoint UI - breakpoint-related operations", in the same commit as the [ngStyle] positioning that is actually used
Usage removed by never used. Only spec cases (#7304, #7988) ever called them

They have been unused for the two years since they were added.

Any related issues, documentation, discussions?

Closes #9110

How was this PR tested?

No new tests. The surviving positioning keeps its cases.

From frontend/:

  • npx ng test --watch=false --include='**/breakpoint-condition-input.component.spec.ts' --include='**/code-debugger.component.spec.ts' --include='**/code-editor.component.spec.ts': 3 files, 126 tests, all pass. The last two specs cover the component's host and the code editor that declares it.
  • npx tsc --noEmit -p src/tsconfig.app.json and -p src/tsconfig.spec.json: clean.
  • yarn format:ci: clean.
  • npx ng build: success.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5.5)

🤖 Generated with Claude Code

BreakpointConditionInputComponent's left() and top() have never had
a production caller: since apache#2953 the popup is positioned by
ngOnChanges through the template's [ngStyle] binding. Remove both
methods and the spec cases that only exercised them.
Copilot AI balanced review requested due to automatic review settings October 23, 2026 21:12
@aglinxinyuan aglinxinyuan self-assigned this Oct 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang
    You can notify them by mentioning @Yicong-Huang in a comment.

@github-actions github-actions Bot added refactor Refactor the code frontend Changes related to the frontend GUI labels Oct 23, 2026
@codecov-commenter

codecov-commenter commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.58%. Comparing base (6cfcfb1) to head (e7590fb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9113      +/-   ##
============================================
+ Coverage     92.34%   92.58%   +0.24%     
+ Complexity     5046     5029      -17     
============================================
  Files          1261     1260       -1     
  Lines         54117    53705     -412     
  Branches       6767     6697      -70     
============================================
- Hits          49973    49723     -250     
+ Misses         2485     2324     -161     
+ Partials       1659     1658       -1     
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø) Carriedforward from 2351cb8
agent-service 99.16% <ø> (ø) Carriedforward from 2351cb8
amber 88.07% <ø> (+0.74%) ⬆️ Carriedforward from 2351cb8
computing-unit-managing-service 60.48% <ø> (ø) Carriedforward from 2351cb8
config-service 87.37% <ø> (ø) Carriedforward from 2351cb8
file-service 82.00% <ø> (ø) Carriedforward from 2351cb8
frontend 96.54% <ø> (-0.02%) ⬇️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 2351cb8
pyamber 98.53% <ø> (-0.05%) ⬇️ Carriedforward from 2351cb8
workflow-compiling-service 72.43% <ø> (ø) Carriedforward from 2351cb8

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aglinxinyuan
aglinxinyuan requested a review from kunwp1 October 23, 2026 22:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change removes confirmed dead code and its dedicated tests without altering active behavior.

No open issues 🎉

Review effort: Lite

@kunwp1
kunwp1 enabled auto-merge October 25, 2026 16:57
@kunwp1
kunwp1 added this pull request to the merge queue Oct 25, 2026
Merged via the queue into apache:main with commit 98b875f Oct 25, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI refactor Refactor the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove the unused left() and top() methods of BreakpointConditionInputComponent

4 participants