-
Notifications
You must be signed in to change notification settings - Fork 808
Cms/workspace extensions #7265
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?
Cms/workspace extensions #7265
Conversation
Hi @hifi-phil Thanks for the PR. |
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.
I know these were authored by an AI agent, but solid improvements. I feel like there could be a bit more clarity around the boundaries and roles of workspace contexts. I think the way it's worded just feels kind of ambiguous, I'd be questioning it if I was reading as an implementor, for sure.
--- | ||
description: Establish an extension to communicate across the application. | ||
description: >- | ||
Learn how to create workspace contexts that provide shared state management and communication between workspace extensions. |
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.
I might consider rephrasing this as "and communication between extensions in the workspace."
My initial impression of the text as-is was that it was saying that contexts managed data inside of the workspace extension, but then that other workspace extensions could also access/interface with the context, too.
Based on text below, that's not the case.
You can add additional Workspace Contexts using the `workspaceContext` Extension Type, which allows you to inject custom logic into your own Workspace or another one. | ||
Workspace Contexts provide: | ||
- **Shared state** scoped to a specific workspace instance | ||
- **Communication layer** between workspace extensions |
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.
Same thing as above - let's clarify if workspace contexts manage data between sub-extensions inside of the workspace, or if they're also able to act as a public interface for extensions inside of other workspaces.
{% endcode %} | ||
|
||
4. Add the following code to the existing `my-element.ts` from the `src`folder: | ||
## View Lifecycle |
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.
Super helpful section, I'm going to try to keep this in mind as I go through other docs, too.
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.
You removed some pages, shouldn't you add redirects according to: https://docs.umbraco.com/contributing/documentation/style-guide/structure#deleting-an-article
Improved wording on workspace context Added redirects to renamed pages
@hifi-phil would you be OK to look into/resolve these merge conflicts please? TIA! |
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.
I got started reviewing this one, but I can see that it's a lot of work to get this one on track/aligned with the rest.
I think there is some content that is just there because it could generate it. But from a consumer perspective I would always aim to keep Docs slim, write what is worth mentioning regarding the topic and not all the general/generic stuff.
With these comments I think there would be the base direction to go through the whole PR and correct it and clean it up.
|
||
# Extension Types: Workspaces | ||
|
||
Workspace extensions provide functionality that operates within specific workspace environments, such as document editing, media management, or member editing. These extensions can communicate through shared workspace contexts to create cohesive, integrated functionality. |
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.
I think if the Docs are here to help people learn and understand concepts, then the texts we provide should be carefully curated to guide people through the most common aspects and push the secondary aspects to more advanced sections.
When I read this sentence, I'm not sure what I was supposed to learn. Maybe my context-window is too short, or the AI just generated some text that sounds nice, but I don't see how this helps would help me or anyone new to understand the topic of this section.
|
||
## Available Extension Types | ||
|
||
Workspace extensions include several types that work together through shared state management: |
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.
after reading this sentence, I have more questions.
"state management?" what does that mean in this context
Footer Apps (Status Monitoring) | ||
``` | ||
|
||
## Getting Started |
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.
This feels like a repetition of the Integration Patterns
above.
I would say Getting Start should properly cover how to get started with you own Workspace from the ground.
And then another Headline could be for context regarding extension an existing Workspace, which is partly covered by the Extension-Types
|
||
## Purpose | ||
|
||
Action Menu Items provide: |
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.
Feels very AI generated, but to be fair its true, but a bit heavy to read. Like do you need to be so spell it out...
- **Progressive disclosure** for related operations | ||
- **Context integration** with workspace state | ||
|
||
{% hint style="info" %} |
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.
This is misplaced, should be together with the Manifest
|
||
## Implementation | ||
|
||
Create a workspace action menu item by extending `UmbWorkspaceActionMenuItemBase` and implementing the `execute` method. This provides the functionality that runs when users select the menu item: |
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.
I think the word select
is off here. I think interacts is better.
|
||
## Action Relationship | ||
|
||
Menu items create dropdown menus for their associated actions: |
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.
Menu items create dropdown menus for their associated actions: | |
Menu items display a dropdown menus for their associated actions: |
// Menu: Save and close, Save as template, Save and publish | ||
``` | ||
|
||
### Destructive Actions |
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.
I dont agree, lets remove this one
// Menu: Delete, Archive, Reset | ||
``` | ||
|
||
### Context-Specific Options |
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.
I also think this example is a bit odd
} | ||
``` | ||
|
||
## Best Practices |
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.
Ah, do we need to spell this out. I mean if we have to cover these topics in all articles it becomes very repetive and a lot of content that does not have much value.
…cms/workspace-extensions
- Updated workspace README intro text to be more concise and clear - Changed "include several types that work together through shared state management" to "can be grouped into these types" - Restored workspace action menu items content after merge 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The merge conflict is now fixed |
@nielslyngsoe I have updated the files as requested |
📋 Description
Fixes #7269
This PR has been generated by AI using Claude Code. First the example in the main UMbraco source was updated with new features, then off the back of this new docs were added and existing ones were improve to match the code base.
I would like to community team to review this and critique the submission. Tell me if its too much, not enough and what constitutes a good article, in addition to the ones already in the guide. This will further allow me to tweak the prompts to make better output.
✅ Contributor Checklist
I've followed the Umbraco Documentation Style Guide and can confirm that:
Product & Version (if relevant)
v16
Deadline (if relevant)
📚 Helpful Resources