Skip to content

fix: drop deprecated prop so no JS error is shown#2228

Merged
stalniy merged 1 commit intoakash-network:mainfrom
jzsfkzm:bugfixes/2105-get-content-anchor-el
Nov 17, 2025
Merged

fix: drop deprecated prop so no JS error is shown#2228
stalniy merged 1 commit intoakash-network:mainfrom
jzsfkzm:bugfixes/2105-get-content-anchor-el

Conversation

@jzsfkzm
Copy link
Contributor

@jzsfkzm jzsfkzm commented Nov 16, 2025

closes #2105

Summary by CodeRabbit

  • Bug Fixes
    • Fixed menu anchoring for select-style checkboxes so dropdowns now rely on default anchoring. This improves dropdown positioning and visual alignment across different layouts and screen sizes, reducing occasional misplacement of menu content.

@jzsfkzm jzsfkzm requested a review from a team as a code owner November 16, 2025 16:40
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 16, 2025

Walkthrough

Removed the getContentAnchorEl: null property from MenuProps in SelectCheckbox.tsx, reverting to default menu anchoring behavior; change targets the runtime exception observed when unselecting all services in the logs filter.

Changes

Cohort / File(s) Summary
Menu anchoring configuration
apps/deploy-web/src/components/shared/SelectCheckbox.tsx
Removed getContentAnchorEl: null from MenuProps, relying on default anchoring (no other MenuProps changes).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-line property removal in one UI component.
  • Pay attention to:
    • SelectCheckbox.tsx — ensure Menu anchoring behaves correctly across browsers.
    • Any callers or tests referencing menu anchor behavior or relying on the previous explicit value.

Poem

🐰 I nudged an anchor, set it free,
The menu popped where it should be.
No more errors, logs can play,
A little hop, and bugs away! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: removing a deprecated property to fix a JavaScript error, which directly aligns with the core objective of fixing the runtime exception.
Linked Issues check ✅ Passed The PR removes the deprecated getContentAnchorEl property from MenuProps, which directly addresses the runtime error described in issue #2105 that occurs when unselecting services.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the runtime exception by removing the deprecated property; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34e2923 and 1cf09e7.

📒 Files selected for processing (1)
  • apps/deploy-web/src/components/shared/SelectCheckbox.tsx (0 hunks)
💤 Files with no reviewable changes (1)
  • apps/deploy-web/src/components/shared/SelectCheckbox.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.25%. Comparing base (b9a6436) to head (1cf09e7).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2228      +/-   ##
==========================================
- Coverage   47.59%   47.25%   -0.34%     
==========================================
  Files        1032     1022      -10     
  Lines       29258    28909     -349     
  Branches     7564     7535      -29     
==========================================
- Hits        13926    13662     -264     
- Misses      14952    14953       +1     
+ Partials      380      294      -86     
Flag Coverage Δ *Carryforward flag
api 82.04% <ø> (ø) Carriedforward from b9a6436
deploy-web 26.41% <ø> (-0.01%) ⬇️
log-collector ?
notifications 87.94% <ø> (ø) Carriedforward from b9a6436
provider-console 81.48% <ø> (ø) Carriedforward from b9a6436
provider-proxy 84.96% <ø> (ø) Carriedforward from b9a6436

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

Files with missing lines Coverage Δ
...eploy-web/src/components/shared/SelectCheckbox.tsx 0.00% <ø> (ø)

... and 66 files with indirect coverage changes

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

width: 250
}
},
getContentAnchorEl: null,
Copy link
Contributor

@stalniy stalniy Nov 17, 2025

Choose a reason for hiding this comment

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

question(blocking): @jzsfkzm could you please explain a bit why we need to remove it? I looked into MenuProps and I don't see this property but there is anchorEl. Should we use it instead?

Copy link
Contributor Author

@jzsfkzm jzsfkzm Nov 17, 2025

Choose a reason for hiding this comment

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

question(blocking): @jzsfkzm could you please explain a bit why we need to remove it? I looked into MenuProps and I don't see this property but there is anchorEl. Should we use it instead?

Sure! With this prop being present, when you click the Select for selecting services, there's a JS warning thrown in Developer Console:
Screenshot 2025-11-17 at 11 08 23
This is not showing in prod for some reason though. Anyway, docs for 5.x suggest just to remove the prop, and anchorOrigin and transformOrigin are well enough for positioning the menu.

@jzsfkzm jzsfkzm force-pushed the bugfixes/2105-get-content-anchor-el branch from 34e2923 to 1cf09e7 Compare November 17, 2025 10:17
@stalniy stalniy merged commit 7efc4a7 into akash-network:main Nov 17, 2025
62 checks passed
stalniy pushed a commit that referenced this pull request Nov 20, 2025
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.

Unselecting service in logs causes runtime exception

2 participants

Comments