Skip to content

IBX-12188: Replaced legacy alerts and notifications with the design-system Alert - #2034

Merged
dew326 merged 11 commits into
6.0from
IBX-12188-alert-component
Sep 11, 2026
Merged

IBX-12188: Replaced legacy alerts and notifications with the design-system Alert#2034
dew326 merged 11 commits into
6.0from
IBX-12188-alert-component

Conversation

@tischsoic

@tischsoic tischsoic commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Warning

Contains a TEMPORARY dependencies.json commit pinning design-system-twig + admin-ui-assets (IBX-12188-alert-component) for browser tests. Remove that commit and the ibexa/admin-ui-assets IBX-12188-alert-component [TMP] branch before merging.

🎫 Issue IBX-12188

Related PRs:

Description:

Legacy alerts and toast notifications now render the design-system Alert:

  • ui/component/alert/alert.html.twig and ui-dev/.../common/alert/alert.js are deprecated adapters over <twig:ibexa:alert> / @ids-components/components/Alert — the existing include/embed call sites across packages keep working (size, show_subtitle_below, attr and the title block are no longer supported). Inline renders get ibexa-alert-adapter for the bottom margin Bootstrap's .alert used to add.
  • Toasts: notification.html.twig is rendered once per type into data-template-<type>; admin.notifications.js / admin.error.page.js initialise the DS Alert behaviour on each toast and dismiss through it (no Bootstrap Alert). ibexa.helpers.notification.* API unchanged.
  • _alerts.scss removed; only ids-alert* classes remain (Behat locators and the invitation-modal JS updated).

Merge order: after design-system + design-system-twig (and an admin-ui-assets bump to a design-system build containing Alert).

For QA:

Login with wrong credentials (inline error alert); publish content (success toast auto-closes after 5 s, error toasts stay until closed); reset-password confirmation; user invitation modal warnings (close hides them); Content type edit "non-translatable fields" warning.

Documentation:

Legacy alert.html.twig params size, show_subtitle_below, attr and the title block are dropped; new code should use <twig:ibexa:alert>.

Comment on lines +3 to +4
// Transitional: keeps the spacing legacy `alert.html.twig` call sites got from Bootstrap's `.alert` margin.
// Goes away together with the adapter template.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

change these comments into TODO: comment

Comment on lines +1 to +6
{#
Compatibility wrapper around the design-system <twig:ibexa:alert> component, kept for the
existing `include`/`embed` call sites. New code should use <twig:ibexa:alert> directly.
Legacy params `size`, `show_subtitle_below` and `attr` are ignored; the `title` block is gone (title is a plain string prop).
Inline renders get `ibexa-alert-adapter`, which restores the bottom margin Bootstrap's `.alert` used to add.
#}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Change into TODO comment

const notificationNode = container.querySelector('.alert');

notificationsContainer.append(notificationNode);
appendNotification(notificationsContainer, { label, message: escapeHTML(message) });

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.

what would you say to escapeHTML inside appendNotification instead of here?

subtitle = null,
size = 'medium',
iconName: iconNameProp = null,
iconName = null,

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.

can we set defaults to empty strings to avoid ?? '' later in code?

@tischsoic
tischsoic force-pushed the IBX-12188-alert-component branch from 9ec4d8c to 1cf9873 Compare September 1, 2026 12:51
@barbaragr
barbaragr force-pushed the IBX-12188-alert-component branch 2 times, most recently from 9fc12fc to b82a69c Compare September 7, 2026 18:54

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

Image

Users/Invite members displays those two boxes by default. user is unable to close them.

@tischsoic
tischsoic requested a review from barbaragr September 10, 2026 06:31
…ystem Alert

Co-Authored-By: Claude <noreply@anthropic.com>
@tischsoic

tischsoic commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

TEMPORARY dependencies.json removed

Recorded for the merge: with the pins in place, on 4bae372bc, all browser tests were greenrun 34455443907 (AdminUI-OSS 0/1, AdminUI-Headless 0/1, AdminUI-Experience/Commerce 0/1, Basic Page Builder tests) plus Frontend build test.

Important

Merge after ibexa/design-system#134 and ibexa/design-system-twig#87. Until the design system changes are released, the frontend build here cannot resolve @ids-components/components/Alert, so expect it red in the meantime.

tischsoic and others added 5 commits September 10, 2026 15:07
Co-Authored-By: Claude <noreply@anthropic.com>
…helper.js

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
tischsoic and others added 5 commits September 10, 2026 15:07
The notification helper now always escapes the message and hands the not yet appended node to onShow, so a caller that needs markup in the message injects it itself. Sub-items, the only user of rawPlaceholdersMap, does that in its existing onShow callback.

Co-Authored-By: Claude <noreply@anthropic.com>
…n helper

The helper is imported by CKEditor plugins bundled in the richtext Encore config, which does not alias @ibexa-design-system, so importing the Alert class there broke that build. The notification entries own the instance now.

Co-Authored-By: Claude <noreply@anthropic.com>
The toast template passed iconPath as a runtime placeholder, so the design-system Alert took the explicit-path branch and the placeholder was replaced with an empty string for every notification without a custom icon, leaving an empty <use>. The template now lets the component pick the icon for its type, and customIconPath is applied to the rendered node instead.

Co-Authored-By: Claude <noreply@anthropic.com>
The forgot-password confirmation carried a copy-pasted empty extra_content
block, which now renders an empty description node and its gap, and it still
overrode the icon with the legacy bare checkmark while the sibling
reset-password alert uses the component default; both are gone. The
reset-password alert is rendered directly rather than through the adapter, so
it gets the missing bottom spacing from the login stylesheet. The notifications
container no longer passes icon_path, which the toast template stopped using.
The message placeholder is replaced through a function so that a message
containing $&, $` or $' is not read as a replacement pattern.

Co-Authored-By: Claude <noreply@anthropic.com>
Both alerts are hidden with a single-class modifier, which used to outrank the
legacy .ibexa-alert rules because those came earlier in the same stylesheet.
The design-system .ids-alert display declaration is emitted after ibexa.scss
(ibexa-ids-assets.scss is the last file of the ibexa-admin-ui-layout-css entry),
so at equal specificity it now wins and the alerts showed on opening the modal
and stayed visible when closed. The modifiers are doubled to outrank it.

Co-Authored-By: Claude <noreply@anthropic.com>
@tischsoic
tischsoic force-pushed the IBX-12188-alert-component branch from 4bae372 to 883520d Compare September 10, 2026 13:09
@sonarqubecloud

Copy link
Copy Markdown

@dew326
dew326 merged commit 7f29c79 into 6.0 Sep 11, 2026
18 of 25 checks passed
@dew326
dew326 deleted the IBX-12188-alert-component branch September 11, 2026 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants