feat: Display container related message - #21900
Conversation
…essages-for-container
…y the required message
taken into account for issue count and navigation
| } | ||
|
|
||
| /** | ||
| * Retrieves warning and error groups of the bound container. |
There was a problem hiding this comment.
| * Retrieves warning and error groups of the bound container. | |
| * Retrieves warning and error message groups of the bound container. |
| const errorMessages: string[] = []; | ||
| messages?.forEach((message) => { | ||
| if (message.severity === Configurator.MessageSeverity.WARNING) { | ||
| errorMessages.push(message.message); |
There was a problem hiding this comment.
That's indeed weird, I had it before in the banner component.
What about renaming errorMessages to warningMessages, warningMessages to infoMessages?
Since it's a new component we can do that. I know that some style elements probably still carry names like 'Error'.
What do you think?
@Larisa-Staroverova
| * @param messages - Messages issued by the configuration engine | ||
| * @returns Messages grouped by the severity they are rendered with | ||
| */ | ||
| export function splitMessagesBySeverity( |
There was a problem hiding this comment.
Can we have this function in a re-use service? That's not that nice for extensibility, imagine someone overrides this component, then still this piece is needed elsewhere
@Larisa-Staroverova
| } | ||
|
|
||
| /** | ||
| * Warning and error groups of the bound container row. |
There was a problem hiding this comment.
| * Warning and error groups of the bound container row. | |
| * Warning and error message groups of the bound container row. |
| </div> | ||
| </div> | ||
| </div> | ||
| <ng-container *ngIf="messages$ | async as messages"> |
There was a problem hiding this comment.
@Larisa-Staroverova font size of error messages does not match
| * is set, container min/max info and required messages are prepended. | ||
| */ | ||
| messages$: Observable<ConfiguratorMessagesView> = | ||
| this.configRouterExtractorService |
There was a problem hiding this comment.
ConfiguratorAttributeProductCardComponent used to receive everything via productCardOptions — a purely presentational component. Now it injects five services and builds its own data stream.
The card is rendered per row, respectively per value. For a bundle attribute with 50 values, that means 50 independent subscriptions to the overall configuration, each of which runs getOptionalGroupById — a recursive tree search — on every configuration change. That is O(cards × groups) per update, and on top of that every card pulls the router data.
The parent component already has the configuration. The row's messages should be passed in via productCardOptions, just like everything else.
There was a problem hiding this comment.
I fully agree! I am working on it.
| */ | ||
| @Injectable({ providedIn: 'root' }) | ||
| export class ConfiguratorMessageService { | ||
| /** |
There was a problem hiding this comment.
ConfiguratorMessageService sits in rulebased/core/facade/ and imports ICON_TYPE from @spartacus/storefront. On develop, rulebased/core has zero storefront imports — so this introduces a new dependency direction, triggered by a service that carries CSS class names, ARIA roles and UI key prefixes. It belongs in components/.
3 participants
No description provided.