Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Add correct Block Type label to Block Workspace editor #2017

Closed
wants to merge 7 commits into from
Closed

Bugfix: Add correct Block Type label to Block Workspace editor #2017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4af3935
Add correct block type label to block workspace editor
rickbutterfield Jun 19, 2024
4e8ce56
Remove `ifDefined` for clearer code
rickbutterfield Jun 19, 2024
50470ac
Changes spaces back to tabs
rickbutterfield Jun 19, 2024
6b82f72
Fix tabs/spaces issue
rickbutterfield Jun 19, 2024
3134388
Fix tabs/spaces issue
rickbutterfield Jun 19, 2024
083396f
Fix tab/space issue
rickbutterfield Jun 19, 2024
ea15e43
Fix whitespace issues
Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/packages/block/block/workspace/block-workspace-editor.element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element';

@customElement('umb-block-workspace-editor')
export class UmbBlockWorkspaceEditorElement extends UmbLitElement {
//

@property({ type: String, attribute: false })
workspaceAlias?: string;

@property({ type: String, attribute: false })
Copy link
Member

Choose a reason for hiding this comment

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

Hi @rickbutterfield

I'm so happy that you are getting your hands dirty and getting into the code.

To prevent, too much prop drilling and hard connections, we encourage using the Context API. So in this case you would consume the Block Workspace Context, and observe the name, which should be available via an observable.

In this way there is a bit of plumbing below in the routes that can be cleaned out :-)

I hope that makes sense :-)
Lets me know what you think and if you have ideas for better DX

label?: string;

render() {
return this.workspaceAlias
? html` <umb-workspace-editor alias=${this.workspaceAlias} headline=${'BLOCK EDITOR'}> </umb-workspace-editor> `
return this.workspaceAlias && this.label
? html` <umb-workspace-editor alias=${this.workspaceAlias} headline=${this.label}> </umb-workspace-editor> `
: '';
}

Expand Down
35 changes: 27 additions & 8 deletions src/packages/block/block/workspace/block-workspace.context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,25 +94,30 @@ export class UmbBlockWorkspaceContext<LayoutDataType extends UmbBlockLayoutBaseM
path: 'create/:elementTypeKey',
component: UmbBlockWorkspaceEditorElement,
setup: async (component, info) => {
(component as UmbBlockWorkspaceEditorElement).workspaceAlias = manifest.alias;

const elementTypeKey = info.match.params.elementTypeKey;
this.create(elementTypeKey);
await this.create(elementTypeKey);

new UmbWorkspaceIsNewRedirectController(
this,
this,
this.getHostElement().shadowRoot!.querySelector('umb-router-slot')!,
);

(component as UmbBlockWorkspaceEditorElement).workspaceAlias = manifest.alias;
const name = this.getName();
(component as UmbBlockWorkspaceEditorElement).label = name;
},
},
{
path: 'edit/:udi',
component: UmbBlockWorkspaceEditorElement,
setup: (component, info) => {
(component as UmbBlockWorkspaceEditorElement).workspaceAlias = manifest.alias;
setup: async (component, info) => {
const udi = decodeFilePath(info.match.params.udi);
this.load(udi);
await this.load(udi);

(component as UmbBlockWorkspaceEditorElement).workspaceAlias = manifest.alias;
const name = this.getName();
(component as UmbBlockWorkspaceEditorElement).label = name;
},
},
]);
Expand Down Expand Up @@ -147,7 +152,13 @@ export class UmbBlockWorkspaceContext<LayoutDataType extends UmbBlockLayoutBaseM
this.observe(
this.#blockManager!.contentOf(contentUdi),
(contentData) => {
this.content.setData(contentData);
if (contentData) {
this.content.setData(contentData);
const blockType = this.#blockManager!.getBlockTypeOf(contentData.contentTypeKey);
if (blockType?.label) {
this.setName(blockType.label);
}
}
},
'observeContent',
);
Expand Down Expand Up @@ -218,6 +229,11 @@ export class UmbBlockWorkspaceContext<LayoutDataType extends UmbBlockLayoutBaseM
throw new Error('Block Entries could not create block');
}

const blockType = this.#blockManager!.getBlockTypeOf(contentElementTypeId);
if (blockType?.label) {
this.setName(blockType.label);
Copy link
Member

@nielslyngsoe nielslyngsoe Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We are still missing the Label interpretation feature, meaning using the Label (template) would not necessary be very pretty.

How was it in v.13, did we show the interpolated label in the header of the modal? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right! The modal label shows the interpolated version of it. 🤦‍♂️ Shall I draft this until that gets resolved?

Copy link
Member

Choose a reason for hiding this comment

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

Well, we need the label soon.. so maybe you could leave those parts commented out and instead have the name set to the Element Type Name?

}

this.#layout.setValue(blockCreated.layout as LayoutDataType);
this.content.setData(blockCreated.content);
if (blockCreated.settings) {
Expand Down Expand Up @@ -277,7 +293,10 @@ export class UmbBlockWorkspaceContext<LayoutDataType extends UmbBlockLayoutBaseM
}

getName() {
return 'block name content element type here...';
return this.#label.value;
}
setName(value: string): void {
Copy link
Member

Choose a reason for hiding this comment

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

I view the Workspace Context as a public API for extensions(here under property editors, workspace views etc), so we would not like any of those to change the name. So in that perspective a setName method should not be available. :-)

It could be an internal one, like #setName but I would be fine by setting the value directly on the label, like this.#label.setValue(...)

this.#label.setValue(value);
}

// NOTICE currently the property methods are for layout, but this could be seen as wrong, we might need to dedicate a data manager for the layout as well.
Expand Down