Skip to content

fix(alert-modal): treat a null title as no title - #4117

Open
idaiv wants to merge 2 commits into
2026.xfrom
fix/alert-modal-null-title
Open

idaiv wants to merge 2 commits into
2026.xfrom
fix/alert-modal-null-title

Conversation

@idaiv

@idaiv idaiv commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Changes in this pull request

Every alert raised through trackError without its own title renders with the icon sitting clear of the message it belongs to:

The cause is a null that slips past the title guard.

trackError's default path hands the error to ErrorModalService, which reports a plain (non-API) error as { title: null } — it has no title to give. useAlertModal only guarded against undefined:

title: !isUndefined(title) ? t(title) : t('error')

so null took the first branch, went through t(), and reached antd as a defined title. antd renders its title slot for any value that is not undefined or null (hasTitle = props.title !== undefined && props.title !== null), so the dialog gets:

  • an empty .ant-modal-confirm-title above the message,
  • .ant-modal-confirm-body-has-title, which aligns the icon to that empty heading rather than to the text,
  • the paragraph's rowGap between the empty heading and the message.

The icon ends up a row above the first line of text. An error raised with a title — every ApiError — was unaffected, which is why the standard error alerts look right and only the plain ones do not.

The guard now uses isNil, so a null title falls back to the same default heading an omitted one already gets. Applied to all four kinds (info, error, warn, success) since they share the pattern, and title is typed string | null to match what callers actually pass.

Additional info

No call site changes: everything that already passed a title behaves exactly as before, and this is the path trackError has always taken.

Covered by use-alert-modal.test.tsx — the null-title cases fail against the old guard and pass against the new one.

Found while looking at plain GeneralError alerts in the backend power tools bundle, where every one of the 19 call sites hits this path.

🤖 Generated with Claude Code

trackError reports a plain error through ErrorModalService as
{ title: null }, and the title guard only covered undefined - null was
passed to t() and handed on to antd. antd renders its title slot for
every value that is not undefined or null, so those alerts came out with
an empty heading above the message, the icon aligned to the heading and
a row gap between the two: the icon sits clear of the text it belongs to.

A null title now falls back to the same default heading an omitted one
gets, so an error raised without its own title renders as the standard
error alert.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@idaiv
idaiv requested review from xIrusux and a balanced review from Copilot September 17, 2026 16:06
@idaiv idaiv added this to the 2026.3.0 milestone Sep 17, 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.

🟢 Approved

The focused, backward-compatible fix covers every shared alert variant with regression tests.

Pull request overview

Fixes null alert titles so default headings render correctly and icons remain aligned.

Changes:

  • Uses isNil for all alert variants.
  • Accepts nullable titles and adds focused regression tests.

Review: Fixes the root cause at the shared hook boundary, covers all variants, and remains backward compatible. No documentation update is needed; remaining risk is minimal.

File summaries
File Description
use-alert-modal.tsx Normalizes nullish titles to alert defaults.
use-alert-modal.test.tsx Tests custom and null-title behavior.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants