Block Editor: Implements hints for Tabs (fixes #21178)#21672
Block Editor: Implements hints for Tabs (fixes #21178)#21672nielslyngsoe wants to merge 10 commits intomainfrom
Conversation
Co-authored-by: nielslyngsoe <6791648+nielslyngsoe@users.noreply.github.com>
Co-authored-by: nielslyngsoe <6791648+nielslyngsoe@users.noreply.github.com>
# Conflicts: # src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/views/edit/block-workspace-view-edit-content-no-router.element.ts # src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/views/edit/block-workspace-view-edit.element.ts
There was a problem hiding this comment.
Pull request overview
Implements tab-level hint/badge support in the Block Workspace by introducing per-tab view contexts and wiring validation hints into those contexts so tab headers can display badges when a tab contains validation issues.
Changes:
- Add per-tab
UmbViewControllerinstances in block workspace edit views and render tab badges based onfirstHintOfVariant. - Switch block element managers from
UmbHintContexttoUmbViewContextand route validation hints throughview.hints. - Wire block workspace
content/settingsviews to inherit from the workspace view (but includes a stray debug subscription).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/views/edit/block-workspace-view-edit.element.ts | Creates/provides per-tab view contexts and renders hint badges in router-based block edit tabs. |
| src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/views/edit/block-workspace-view-edit-content-no-router.element.ts | Adds similar per-tab view contexts + hint badge rendering for inline/no-router block editing. |
| src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace.context.ts | Hooks up view inheritance for content/settings views (but also adds a debug console subscription). |
| src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-element-manager.ts | Replaces hint context with view context and connects validation-to-hints to view.hints. |
Comments suppressed due to low confidence (1)
src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/views/edit/block-workspace-view-edit-content-no-router.element.ts:71
- When
_hasRootPropertieschanges, you don’t call#setupViewContexts(). If root properties become available after initial load, the required view context (and its hint observer) will never be created. Call#setupViewContexts()from thehasPropertiesobserver as well.
this.observe(
this.#tabsStructureHelper.hasProperties,
(hasRootProperties) => {
this._hasRootProperties = hasRootProperties;
this.#checkDefaultTabName();
},
'observeRootProperties',
);
|
|
||
| if (viewAlias === null) { | ||
| // for the root tab, we need to filter hints | ||
| view.hints.setPathFilter((paths) => paths[0].includes('tab/') === false); |
There was a problem hiding this comment.
The path filter assumes paths[0] exists. With the new hint path prefix of [] in the block element manager, root properties (no container) can produce hints with an empty path, causing paths[0].includes(...) to throw. Use a safe check (e.g., handle empty paths as “not in a tab”).
| view.hints.setPathFilter((paths) => paths[0].includes('tab/') === false); | |
| view.hints.setPathFilter((paths) => { | |
| const firstPath = paths[0]; | |
| // Treat empty paths as "not in a tab", so they belong to the root tab | |
| if (!firstPath) { | |
| return true; | |
| } | |
| return firstPath.includes('tab/') === false; | |
| }); |
|
|
||
| if (viewAlias === null) { | ||
| // for the root tab, we need to filter hints, so in this case we do accept everything that is not in a tab: [NL] | ||
| view.hints.setPathFilter((paths) => paths[0].includes('tab/') === false); |
There was a problem hiding this comment.
The root-tab hint filter assumes paths[0] exists. Root-property validation hints can have an empty path (no container), which will make paths[0].includes(...) throw at runtime. Please guard for empty paths (treat them as root/non-tab hints).
| view.hints.setPathFilter((paths) => paths[0].includes('tab/') === false); | |
| view.hints.setPathFilter((paths) => { | |
| const firstPath = paths[0]; | |
| // Root-property validation hints can have an empty path (no container), so treat them as root/non-tab hints. | |
| if (!firstPath) { | |
| return true; | |
| } | |
| return firstPath.includes('tab/') === false; | |
| }); |
| this.view.hints.hints.subscribe((hints) => { | ||
| console.log('workspace', hints); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Remove the debug subscription/console.log. The subscribe here is never unsubscribed (potential leak) and will spam the console in production builds.
| this.view.hints.hints.subscribe((hints) => { | |
| console.log('workspace', hints); | |
| }); |
| this.observe(this.#tabsStructureHelper.childContainers, (tabs) => { | ||
| this._tabs = tabs; | ||
| this.#checkDefaultTabName(); | ||
| this.#setupViewContexts(); | ||
| }); |
There was a problem hiding this comment.
#setupViewContexts() can run before the block workspace context has been consumed (and before #blockManager is set). Since #createViewContext throws when #blockManager is missing, this can crash during initial render. Guard #setupViewContexts/#createViewContext until #blockManager is available, or call #setupViewContexts only after the context callback runs.
| // Create view contexts for root groups | ||
| if (this._hasRootGroups) { |
There was a problem hiding this comment.
The root tab is rendered when _hasRootGroups || _hasRootProperties, but #setupViewContexts() only creates the root view context when _hasRootGroups is true. This means root-property hints (and default root selection) won’t work when there are root properties but no root groups. Create the root view context when _hasRootGroups || _hasRootProperties.
| // Create view contexts for root groups | |
| if (this._hasRootGroups) { | |
| // Create view contexts for root groups/properties | |
| if (this._hasRootGroups || this._hasRootProperties) { |
| #checkDefaultTabName() { | ||
| if (!this._tabs || !this.#blockWorkspace) return; | ||
|
|
||
| // Find the default tab to grab | ||
| if (this._activeTabKey === undefined) { | ||
| if (this._hasRootGroups || this._hasRootProperties) { | ||
| this._activeTabKey = null; | ||
| const context = this.#tabViewContexts.find((context) => context.viewAlias === null); | ||
| if (context) { | ||
| this._activeTabKey = null; | ||
| this.#provideViewContext(null); | ||
| } | ||
| } else if (this._tabs.length > 0) { | ||
| const tab = this._tabs[0]; | ||
| this._activeTabKey = tab.ownerId ?? tab.ids[0]; | ||
| if (tab) { | ||
| this._activeTabKey = tab.ownerId ?? tab.ids[0]; | ||
| this.#provideViewContext(getViewAliasForTab(tab)); | ||
| } | ||
| } |
There was a problem hiding this comment.
#checkDefaultTabName() now depends on #tabViewContexts being populated, but it runs before #setupViewContexts() (and #setupViewContexts() doesn’t re-run #checkDefaultTabName() afterward). This can leave _activeTabKey undefined so no tab content renders until user interaction. Create view contexts before choosing defaults, or re-run default selection after contexts are created.
Implements hints for tabs of the Block Workspace. Notice how this is used for all Blocks, in the Modal when editing Blocks, and when editing List Blocks in inline-mode.
This generally finished the implementation of badges, it seems there where a few missing links/bindings between the View Contexts.
Here a few screenshots of the scenarios I have tested: