-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add folder workspace icon #19366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add folder workspace icon #19366
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…om/umbraco/Umbraco-CMS into v16/feature/content-workspace-icon
@madsrasmussen I love it, it seems done? so should we get it in? |
@nielslyngsoe I found a small hiccup yesterday. The file system folders should have a readonly name field, but after I have changed it to use the I am trying to think of a good generic way to make the name readonly. Maybe something with guards. 🤔 |
@nielslyngsoe This PR has been updated to also work correctly for file system workspaces. Please see description for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR standardizes folder workspaces by introducing a shared editor element with an icon, adds a name write guard to prevent renames on file system folders, and updates all folder workspace editors to use the new shared component.
- Introduced
UmbFolderWorkspaceEditorElement
to render folder workspaces with a consistent icon and name field. - Replaced custom rendering in various folder workspace editor elements to use the shared folder workspace editor.
- Added
UmbNameWriteGuardManager
and applied rename-prevention rules for file system folders in relevant contexts.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
templating/stylesheets/tree/folder/workspace/stylesheet-folder-workspace-editor.element.ts | Added name write guard rule and switched to shared folder workspace editor. |
templating/scripts/tree/folder/workspace/script-folder-workspace-editor.element.ts | Added name write guard rule, switched to shared editor, and updated import path. |
templating/partial-views/tree/folder/workspace/partial-view-folder-workspace-editor.element.ts | Added name write guard rule and switched to shared folder workspace editor. |
media/media-types/tree/folder/workspace/media-type-folder-editor.element.ts | Switched to shared folder workspace editor component. |
documents/document-types/tree/folder/workspace/document-type-folder-editor.element.ts | Switched to shared folder workspace editor component. |
documents/document-blueprints/tree/folder/workspace/document-blueprint-folder-editor.element.ts | Switched to shared folder workspace editor component. |
data-type/tree/folder/workspace/data-type-folder-editor.element.ts | Switched to shared folder workspace editor component. |
core/workspace/namable/name-write-guard.manager.ts | Introduced UmbNameWriteGuardManager to manage rename-permission rules. |
core/workspace/namable/namable-workspace-context.interface.ts | Added nameWriteGuard to the namable workspace context interface. |
core/workspace/namable/index.ts | Exported the new name write guard manager. |
core/workspace/entity-detail/entity-named-detail-workspace-base.ts | Integrated UmbNameWriteGuardManager into entity-named-detail workspaces with default fallback. |
core/workspace/components/workspace-header-name-editable/workspace-header-name-editable.element.ts | Observes write guard rules to disable the name input when renaming is prevented. |
core/tree/folder/workspace/index.ts | Registered the new shared folder workspace editor element. |
core/tree/folder/workspace/folder-workspace-editor.element.ts | Created the shared folder workspace editor with icon styling. |
core/tree/folder/index.ts | Exported the workspace folder module. |
Comments suppressed due to low confidence (1)
src/Umbraco.Web.UI.Client/src/packages/templating/partial-views/tree/folder/workspace/partial-view-folder-workspace-editor.element.ts:14
- [nitpick] The message text casing for 'Partial View folder' should be consistent with other folder types (e.g., capitalize or lowercase 'script folder', 'stylesheet folder') to maintain a uniform user experience.
message: 'It is not possible to change the name of a Partial View folder.',
@@ -1,49 +1,30 @@ | |||
import { UMB_SCRIPT_FOLDER_WORKSPACE_CONTEXT } from './script-folder-workspace.context-token.js'; | |||
import { css, html, customElement, state } from '@umbraco-cms/backoffice/external/lit'; | |||
import { UMB_SCRIPT_FOLDER_WORKSPACE_CONTEXT } from '../../../constants.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import path for UMB_SCRIPT_FOLDER_WORKSPACE_CONTEXT appears inconsistent with other folder workspace editors. It should likely import from './script-folder-workspace.context-token.js' to reference the correct context token.
import { UMB_SCRIPT_FOLDER_WORKSPACE_CONTEXT } from '../../../constants.js'; | |
import { UMB_SCRIPT_FOLDER_WORKSPACE_CONTEXT } from './script-folder-workspace.context-token.js'; |
Copilot uses AI. Check for mistakes.
this.observe( | ||
this.#workspaceContext?.nameWriteGuard.isPermittedForName(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing an optional chain to observe
may result in registering an undefined observable if #workspaceContext
is not yet set. Guard against a missing context before calling observe
.
this.observe( | |
this.#workspaceContext?.nameWriteGuard.isPermittedForName(), | |
if (!this.#workspaceContext) return; | |
this.observe( | |
this.#workspaceContext.nameWriteGuard.isPermittedForName(), |
Copilot uses AI. Check for mistakes.
export class UmbFolderWorkspaceEditorElement extends UmbLitElement { | ||
override render() { | ||
return html`<umb-workspace-editor> | ||
<umb-icon id="icon" slot="header" name="icon-folder"></umb-icon> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using a hardcoded id="icon"
can lead to duplicate IDs if multiple folder workspaces are rendered. Consider using a CSS class or dynamically generated identifier instead.
<umb-icon id="icon" slot="header" name="icon-folder"></umb-icon> | |
<umb-icon class="icon" slot="header" name="icon-folder"></umb-icon> |
Copilot uses AI. Check for mistakes.
export class UmbFolderWorkspaceEditorElement extends UmbLitElement { | ||
override render() { | ||
return html`<umb-workspace-editor> | ||
<umb-icon id="icon" slot="header" name="icon-folder"></umb-icon> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icon lacks an accessible label or aria-hidden
attribute. If it's purely decorative, add aria-hidden="true"
; otherwise, provide an appropriate aria-label
for screen readers.
<umb-icon id="icon" slot="header" name="icon-folder"></umb-icon> | |
<umb-icon id="icon" slot="header" name="icon-folder" aria-hidden="true"></umb-icon> |
Copilot uses AI. Check for mistakes.
This PR adds a workspace icon for all folders.
It also aligns how we render a folder by introducing an element we can use across all workspaces. We should add a 'kind' for this element at some point.
Currently, it is not possible to change the name of a file system folder, which means that the name field should be read-only in this case. The PR also introduces a
UmbNameWriteGuardManager
that can be used within all namable workspaces. The Partial View, Stylesheet, and Script workspaces add a rule to this manager to prevent the name from being edited.