(feat)O3-4564: Support for single extension slot #1400
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Size Change: -166 kB (-2.52%) Total Size: 6.4 MB
ℹ️ View Unchanged
|
chibongho
commented
Jun 18, 2025
| } | ||
| } | ||
|
|
||
| extensionInternalStore.subscribe((internalStore) => { |
Contributor
Author
There was a problem hiding this comment.
This pattern of passing in the new store state to the updateExtensionOutputStore should be unnecessary. As a test, I toggled the rde feature flag and confirmed that the patient charts visit notes workspace shows / hides the visit context switcher accordingly.
| state?: Record<string, unknown>; | ||
| } | ||
|
|
||
| export interface OldExtensionSlotBaseProps { |
Contributor
Author
There was a problem hiding this comment.
Identical to ExtensionSlotBaseProps. Removing
| export interface ExtensionSlotBaseProps { | ||
| name: string; | ||
| /** @deprecated Use `name` */ | ||
| extensionSlotName?: string; |
Contributor
Author
There was a problem hiding this comment.
This is several years old. Removing.
ibacher
reviewed
Jun 18, 2025
| * used to indicate specific configuration. (For example, given a extension with name 'foo', | ||
| * then 'foo', 'foo#bar', 'foo#baz' are all valid extensionIds) | ||
| */ | ||
| export function SingularExtensionSlot({ |
Member
There was a problem hiding this comment.
Maybe we could just name this "RenderExtension"? It's not really a "slot" in the normal sense. Barring that, maybe "SingleExtensionSlot" would be better?
Contributor
Author
There was a problem hiding this comment.
Let's go with "SingleExtensionSlot". I'd rather keep 'Slot' as part of the name, as:
- it's still kind of a slot, in line with the analogy that "a slot is a place (to render extensions)".
- It has a slot name, even though it's hard-coded to 'global' for now. I'm not sure if we want to allow the slot name to be something else. (For example, should workspaces be rendered in a slot named 'workspace' instead of 'global'?)
- Confusingly, we already have an external-facing
<Extension>component, which is used inside<ExtensionSlot>but can include other styling surrounding it.
| for (let id of assignedIds) { | ||
| const { config: extensionConfig } = getExtensionConfigFromStore(extensionConfigStoreState, slotName, id); | ||
| for (let id of extensionIds) { | ||
| const { config: extensionConfig } = getExtensionConfigFromStore(extensionStoreState, slotName, id); |
Member
There was a problem hiding this comment.
Did you mean to change the store here?
Contributor
Author
There was a problem hiding this comment.
It's a confusing rename. I'll change it.
| type SingularExtensionSlotProps = Omit<ExtensionSlotProps, 'name' | 'select'> & { extensionId: string }; | ||
|
|
||
| /** | ||
| * A special extension slot, with slot name 'global', that renders only one single extension |
Member
There was a problem hiding this comment.
It would be nice to include a usage example here as we do with <ExtensionSlot />, especially as this looks like it supports the same feature set.
chibongho
changed the title
Contributor
|
@chibongho Thanks for working on this issue. Could you kindly fix the merge conflicts, please? |
jwnasambu
changed the title
Contributor
Author
|
hm... I'm not able to reproduce the test failure by doing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
This PR introduces a new component
<SingularExtensionSlot>, which allows us to render a specific extension by its name or id.Testing done:
<SingularExtensionSlot>. Also manually used the component and verified that it renders as expected.Screenshots
Related Issue
https://openmrs.atlassian.net/browse/O3-4564
Other
I worked on this because I want to use it as part of the Workspace work. Currently, workspaces are kind of extensions, but they are rendered at the single-spa level. With this work, we should be able to render a particular workspace at the extension level.