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.

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

workspace routable kind #1455

Merged
merged 29 commits into from Mar 21, 2024

Conversation

nielslyngsoe
Copy link
Member

@nielslyngsoe nielslyngsoe commented Mar 19, 2024

Important

This is a breaking change.

If you use workspace extension type, and have implemented your manifest with an element served via the js property on the manifest. Then you are now forced to bring an API(Context-API) as part of that JS file. (read further to see example on how that can be achieved)

You will experience a problem if your manifest looks like this and the js file only exports an element:

{
	type: 'workspace',
	name: '...',
	alias: '...',
	js: () => import('./my-workspace.element.js'),
	meta: {
		entityType: '...',
	},
}

If you dont like to bring an API, you should switch to use the element property — This will work cause we now provide a default workspace context api in this setup.

{
	type: 'workspace',
	name: '...',
	alias: '...',
	element: () => import('./my-workspace.element.js'),
	meta: {
		entityType: '...',
	},
}

If you already have a API but initialised via the Element Web Component, then you should stop doing so, and instead provide your Context in the api property. (Or export both as api and element modules as part of the js property.)
If your element directly uses the context, then you will receive it via the api property. Which you can turn into a getter method to actively react on the receiving of it.

like this example:

{
	type: 'workspace',
	name: '...',
	alias: '...',
	element: () => import('./my-workspace.element.js'),
	api: () => import('./my-workspace.context.js'),
	meta: {
		entityType: '...',
	},
}

or keep it in one file:

like this example:

{
	type: 'workspace',
	name: '...',
	alias: '...',
	js: () => import('./my-workspace.element-and-context.js'),
	meta: {
		entityType: '...',
	},
}

my-workspace.element-and-context.js would then look something like this:

...
export { MyWorkspaceElement as element };
export { MyWorkspaceContext as api };

Description of PR:

Adding and implementing a Workspace Kind called 'routable'.

Generally aligning workspaces even more and removing duplicated code.

In detail:
This gives a default element with a router for workspaces, which as well initialises the workspaceContexts.
This enables using the extension-slot-with-api to become aligned with other extensions coming with element and API.
And then a lot of workspaces now does not have a element anymore.
This also means the routing have been moved to the context of a workspace, only relevant for that kind, so this does not give any breaking changes.

I also got to fix some issues on Block Workspace, and had to out comment a condition as it did not work when creating blocks, need to revisit in a later PR. For now that got out commented.

This also includes the changes to extension-slot-with-api, so it accepts a default-api. plus implementing a default workspace context, so there is something for those who do not like to provide one.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Motivation and context

How to test?

Screenshots (if appropriate)

Checklist

  • If my change requires a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

@nielslyngsoe nielslyngsoe marked this pull request as ready for review March 20, 2024 13:40
@leekelleher leekelleher self-requested a review March 20, 2024 13:51
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

Tested this out. All workspaces are working as expected, except for the MemberType and DocumentBlueprint workspaces, as they threw JS errors and don't load. (I'm aware that DocumentBlueprint is currently in development.)

Extension of alias "Umb.Workspace.MemberType" did not succeed creating an Element with Api, 
Api was created but the Element was missing a JavaScript file via the 'element' or the 'js' property. 
Alternatively define a Element Name in 'elementName' in the manifest.

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

@nielslyngsoe
Copy link
Member Author

@leekelleher cool, thanks for finding that. I have fixed the Member Type.
Blueprint workspace is not done, neither touched as art of this PR :-)

@nielslyngsoe
Copy link
Member Author

@leekelleher Oh, I realised that a few workspaces not would work properly. As this PR requires a Workspace Extension to bring an API.

We might miss a default workspace api fallback solution. But on the other hand I also think it makes sense that you are forced to bring an API. As many things use the Context API to get the Workspace Context. So this is a way for the developer to be enforced to always bring an API. aka. Workspace Context.

I have updated the missing ones, but this then is a breaking change as of now.

@leekelleher leekelleher self-requested a review March 20, 2024 19:38
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

@nielslyngsoe
Copy link
Member Author

@leekelleher okay, so now I'm more clear in what we can do to make it as little breaking as possible. So if one does not provide an API but provides their element via js property, then that would require the js files to bring the API as well. If they provide is via element then it will use a fallback api.
This is thought the most common implementation ive seen. as the js prop was the original one, and then later we extended with element and api. But I think it makes sense to require the js prop brings the api if thats served.
let me know what you think :-)

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

@leekelleher
Copy link
Member

@nielslyngsoe Makes sense, (re: js, element and api) 👍 I'll give it a re-test now.

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

Re-tested backoffice, all working as expected! Super Tak! 🎉

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://ambitious-stone-0033b3603-1455.westeurope.1.azurestaticapps.net

@nielslyngsoe nielslyngsoe merged commit 39fad49 into main Mar 21, 2024
6 checks passed
@nielslyngsoe nielslyngsoe deleted the feature/workspace-kinds-for-routable-workspace branch March 21, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants