Skip to content

fix(alerts): remove double border on InputNumber fields in report modal#42090

Merged
sadpandajoe merged 2 commits into
masterfrom
claude/alerts-reports-selector-box-4c63dd
Jul 16, 2026
Merged

fix(alerts): remove double border on InputNumber fields in report modal#42090
sadpandajoe merged 2 commits into
masterfrom
claude/alerts-reports-selector-box-4c63dd

Conversation

@sadpandajoe

@sadpandajoe sadpandajoe commented Jul 15, 2026

Copy link
Copy Markdown
Member

SUMMARY

The Screenshot width field (and the alert Value/threshold field) in the Alert/Report modal rendered a "box within a box": a bordered input nested inside the already-bordered InputNumber control, with native browser number spinners layered on top of antd's own step handles.

Root cause: both fields passed type="number" to antd's InputNumber. antd forwards unrecognized props to the inner <input>, so it became <input type="number">. That matched a rule in the modal's StyledInputContainer styled component:

input[type='number'] {
  padding: ...;
  border: 1px solid ${theme.colorBorder};
  border-radius: ${theme.borderRadius}px;
}

which gave the inner <input> its own border/padding on top of the wrapping .ant-input-number border — hence the nested box. The type prop was redundant anyway: InputNumber already restricts input to numeric values.

The fix removes type="number" from both InputNumber instances so the component owns its single border. Other type="number" usages in the codebase are on antd's plain Input (a single bordered element) and are unaffected.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Rendered with the project's antd version and the exact StyledInputContainer CSS:

  • Before:
Screen.Recording.2026-07-15.at.2.05.45.PM.mov
  • After:
Screen.Recording.2026-07-15.at.10.21.43.PM.mov

TESTING INSTRUCTIONS

  1. Go to Settings → Alerts & Reports and create a new report.
  2. Under Report Contents, note the Screenshot width field.
  3. Confirm it renders as a single input (no nested box) and shows only antd's up/down step handles on hover — no duplicated native browser spinners.
  4. Repeat for a new alert's Value (threshold) field under Alert Condition.

Unit tests: AlertReportModal.test.tsx passes (68/68).

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

FEATURE FLAGS

FEATURE_ALERT_REPORTS=true
FEATURE_ALERT_REPORT_TABS=true
FEATURE_ALERT_REPORTS_FILTER=true

The screenshot width and alert threshold fields passed type="number" to
antd's InputNumber. antd forwards unknown props to the inner <input>, so it
rendered as <input type="number"> and matched the
StyledInputContainer rule input[type='number'] { border: 1px solid ... },
drawing a bordered box inside the already-bordered .ant-input-number wrapper
(a "box within a box") and adding native spinners on top of antd's step
handles. Removing the redundant prop lets InputNumber own its single border.

Update two toHaveValue assertions accordingly: without type="number",
jest-dom reads the value as a string.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.08%. Comparing base (beb9d53) to head (17b6c48).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #42090   +/-   ##
=======================================
  Coverage   65.08%   65.08%           
=======================================
  Files        2747     2747           
  Lines      153926   153926           
  Branches    35304    35304           
=======================================
  Hits       100186   100186           
  Misses      51828    51828           
  Partials     1912     1912           
Flag Coverage Δ
javascript 70.51% <ø> (ø)

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sadpandajoe sadpandajoe added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Jul 15, 2026
@github-actions github-actions Bot added 🎪 2b10149 🚦 building 🎪 ⌛ 48h Environment expires after 48 hours (default) and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime is building environment on GHA for 2b10149

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 2b10149

Environment: http://34.217.126.139:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@sadpandajoe sadpandajoe added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Jul 15, 2026
@github-actions github-actions Bot added 🎪 2b10149 🚦 building and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Jul 15, 2026
@netlify

netlify Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 17b6c48
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a5863cfd3feca0008af6474
😎 Deploy Preview https://deploy-preview-42090--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 17b6c48

Environment: http://100.23.169.161:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@sadpandajoe
sadpandajoe marked this pull request as ready for review July 16, 2026 05:20
@dosubot dosubot Bot added alert-reports Namespace | Anything related to the Alert & Reports feature change:frontend Requires changing the frontend labels Jul 16, 2026

@aminghadersohi aminghadersohi 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.

Verified both edits are on antd InputNumber (not plain Input) at the threshold and screenshot-width fields — type="number" was inert there and only leaked onto the inner native input, colliding with the StyledInputContainer input[type='number'] border rule. min/max props are untouched, no InputNumber+type="number" combo remains in the file, and the test changes correctly assert the fix. CI green, clean root-cause fix — approving.

@bito-code-review

bito-code-review Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #ddaaa1

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 2b10149..17b6c48
    • superset-frontend/src/features/alerts/AlertReportModal.test.tsx
    • superset-frontend/src/features/alerts/AlertReportModal.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@sadpandajoe
sadpandajoe merged commit 3b7647e into master Jul 16, 2026
116 checks passed
@sadpandajoe
sadpandajoe deleted the claude/alerts-reports-selector-box-4c63dd branch July 16, 2026 17:19
@github-actions github-actions Bot removed 🎪 17b6c48 🚦 running 🎪 ⌛ 48h Environment expires after 48 hours (default) labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alert-reports Namespace | Anything related to the Alert & Reports feature change:frontend Requires changing the frontend size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants