Proposal to change O3 workspace groups

Currently, the workspace system allows for multiple workspaces to be opened simultaneously (with one active and the rest minimized.) Minimized workspaces are useful in that it allows for unsaved changes to forms in the workspace while the user navigates to other workspaces or to different pages (say, different tabs within the patient chart) to look at data. The workspace system has 2 ways to prevent unsaved changes in opened workspaces from getting lost:

  1. The workspace system has a concept of a workspace group; all opened workspaces must belong to the same active workspace group. If there is an attempt to open another workspace of a different workspace group, then the user is presented with a warning modal to discard any unsaved changes to the currently opened workspaces. There are 2 ways to define workspace group(s) for a given workspace:

    a. When defining a workspace in routes.json file, we can optionally specify the workspace group(s) that it belongs to.

    b. Conversely, we can also declare a workspace group in a routes.json file and list of workspaces that are members of the workspace group.

  2. The workspace system has a concept of a contextKey. From here: “The context key is a string that appears in the URL, which defines the pages on which workspaces are valid. If the URL changes in a way such that it no longer contains the context key, then all workspaces will be closed”. The contextKey is passed in to the <WorkspaceContainer> component. When the user attempts to naviagtes to a URL that no longer matches the context keys, they will also get a warning modal to discard unsaved changes.

Thus, we get “unsaved changes” prompt when we attempt to either naviate to a different URL or try to open workspace from another workspace group. However, I think there are navigations that are not necessarily URL changes that should also prompt for unsaved changes. Here’s my use case:

  • For RDE, the vitals / visit notes / forms / order basket workspaces have a concept of the “Visit context”. For example, when creating a visit note, we set the visit context to the visit we want to add the visit note to. (For point of care, we set the visit context to the current active visit; for RDE, we select a past visit instead.) We are also starting to add ability to edit existing encounters. In particular, the visits table or the encounters table in the patient chart allows you to view / edit existing encounters across different visits, without navigating to differen tpages. When editing an existing encounter, the visit context should automatically switch to the visit associated with the encounter. However, we should only allow that to happen when there is no pending changes in existing opened workspaces.

Current, when we launch a workspace to have it opened in a specified workspace group, we do this:

launchWorkspaceGroup('ward-patient', {
  workspaceToLaunch: {
      name: 'ward-patient-workspace',
  },
});

I propose that we solve the issue with the unsaved changes prompts when switching visit contexts by having a context key pass in to `launchWorkspaceGroup, something like this:

launchWorkspaceGroup('patient-chart-forms', {
  workspaceToLaunch: {
      name: 'visit-note-workspace',
  },
  contextKey: <visitUuid>,
  URLKey: `patient/${patientUuid}`
});

Then, the workspace system shows the unsaved changes prompt if we attempt to do launchWorkspaceGroup either:

  • naviagte to a URL that does not match the URLKey
  • call launchWorkspaceGroup with a different group name, or
  • call launchWorkspaceGroup with the same group name, with different contextKey.

Let me know what you think. Thanks!

2 Likes

I thought about this more and created this ticket for it

O3-4705 - Add support for workspace context and workspace group context

Basically, I want to introduce the concept of workspace context and workspace group context. I’m leaving the URL-sensitive contextKey in the <WorkspaceContainer> as is for now.

A more concrete proposal now that I have played around with workspaces more:

  • Add workspace context and workspace group context.

    Currently, we prompt for unsaved changes when we attempt to either:

    1. open a new workspace group that’s different from the current one, or
    2. open a new workspace of type X, when there is already another workspace of type X opened.

    As mentioned in the above post, this is insufficient for some scenarios. We introduce the concept of workspace context and workspace group context. When launching a workspace group, we may specify a workspace group context. When we launch a workspace, we may specify both a workspace context and a workspace group context. When we attempt to launch a workspace, we additionally check for the workspace and workspace group contexts, and prompt for unsaved changes if either changes.

  • Lessen the distinction between “workspace” and “workspace type”.

    When declaring a workspace in a routes.json file, we can specify a "type" attribute. The attribute identifies the workspace action menu icon corresponding to the workspace, so that the icon gets highlighted when the workspace is opened. For example, in patient chart, VisitNoteActionButton is an ActionMenuButton with type='visit-note', and it gets highlighted when we open the visit-notes-form-workspace workspace, because in its declaration, its "type": "visit-note" matches the type of the button.

    Currently, the workspace system imposes a constraint that all open workspace must have both unique workspace names and unique workspace type. The distinction between “workspace name” and “workspace type” is primarily useful when we have “child workspaces”, i.e. a workspace launched from another workspace. For example, in the order basket, the add-drug-order and add-lab-order forms are child workspaces opened from the order-basket workspace, and they all declared with "type": "order". However, “child workspaces” is not an actual supported feature, and we emulate it in a hacky way: when navigating from the main order basket to the drug order form, we do so by closing the order-basket workspace and immediately opening the add-drug-order workspace.

    With the introduction of contexts, this could get unwieldy. I propose that we do away with “child workspaces”, and stop declaring multiple workspaces with the same "type". Instead, we have just one workspace for a workspace "type", and we let it handle its own navigation state for different “pages” (ex: main order basket and drug order form) within the workspace.

  • Only show workspace action menu when we do launchWorkspaceGroup.

    Currently, there are 2 slightly different ways to define the icons that shows up in the workspace action menu. The icons are extensions in the extension slot action-menu-${X}-items-slot:

    • if a workspace group is opened, then X is the workspace group name
    • else X is the name of the current module that’s mounting the <WorkspaceContainer>.

    I propose that we have the workspace action menu icons be completely defined by the workspace group. In other words, we only see the workspace action menu if we do launchWorkspaceGroup(), and they get removed when we do closeWorkspaceGroup(). (This also means we can deprecate the showSiderailAndBottomNav param of <WorkspaceContainer>)

    • For the patient chart in particular, this means we immediately call launchWorkspaceGroup when we open the patient chart, and call cloesWorkspaceGroup when we unmount. This is good, as we should be forced to define the workspace group context (which, in this case, is the visit uuid of the visit in context) for the visit note / clinical forms / order basket workspaces.
  • Add proper support for restoring minimized workspaces.

    Some workspaces, like the patient chart’s visit note workspace, can be hidden / minimized without completely closing. We can restore / un-hide the workspace by clicking on the corresponding workspace action menu icon. This is implemented incorrectly right now; clicking on the icon calls launchWorkspace, replacing any props passed into the existing workspace. This is especially bad with the introduction of workspace context (which is a param of launchWorkspace). Instead, we should make the action menu icon’s onClick do a “restore” action (by un-hiding the existing workspace), and only call launchWorkspace if no workspace of that type is currently opened.

cc: @mseaton @mogoodrich @ibacher @dkigen @vasharma05 @jayasanka

Maybe you can clarify something that’s been bothering me: what is a workspace group? As you’ve said, there’s this “type” feature that was sort of intended to group multiple workspaces that are logically together.

We also need to consider how we keep the correct “state” for each workspace that is “open” live.

We may need to just completely rewrite the whole system.

what is a workspace group? As you’ve said, there’s this “type” feature that was sort of intended to group multiple workspaces that are logically together.

(I don’t love the nomenclature, but…)

A workspace “type” maps to one action menu icon. In the order basket, the main order-basket workspace, and add-lab-order workspace and add-drug-order workspace all have the same type = "order", and so have same action menu icon. See screenshot:

I think what this implies, is that we should never actually launch a particular workspace directly. Rather we should be launching a particular workspace type, with enough props / params passed in so it knows which workspace within the workspace type you intend to open. This is, of course, very incompatible with how we launch workspaces right now . But I think we can sidestep this problem if we make it so that no 2 workspaces have the same workspace type. For example, with the order basket, we can re-implement it with just one workspace and let it manage its own navigation state to the lab order / drug order forms. More aggressively, we can even deprecate the idea of workspace type, by using the workspace name to directly map to an action menu icon, although it breaks backwards incompatibility.

(Aside: what I’ve been proposing as workspace context would be more correctly named as “workspace type context”, but again, we can sidestep this if we don’t have workspace type.)

A workspace group, as is currently defined, is (also) a group of related workspace, although it would be more correct to define it instead as a group of related “workspace types”. (Again, we can sidestep this.) In the ward app, when you click on a patient card, you not only open a workspace, but also open a workspace group, and the action icons for “transfer”, “discharge”, “order basket” etc… all belong to the same workspace group. See screenshot:

Currently, you can have multiple workspaces open, but only if they all belong to the same workspace group. (Contrast with workspace type, where you can only have one workspace of a particular type opened at a time)

We also need to consider how we keep the correct “state” for each workspace that is “open” live

We may need to just completely rewrite the whole system.

Looking at the unit tests, I think the current workspace system does a reasonable job maintaining state for each open workspace. We are using it incorrectly in some places, (like this issue), but I think it’s fixable.

If we eliminate “workspace type”, and also get rid of <WorkspaceContainer> (which my proposal above should help with, as it remove some params we currently need to pass into it), then I think the workspace system is actually alright.

I see. So basically, my read of this situation is that the workspace system is a little incoherent because we have sort of two conceptions of this system is supposed to work and we’re trying to mash them together into one thing.

As I see it, a “Workspace Group” feels like a concept that should be represented more similar to how we do an “Extension Slot” and that makes me think that we should actually make the group and it’s state properties that are passed in to something like the WorkspaceContainer rather than being something that is “launched”. We could then have basically an “empty” mode where nothing gets rendered if that’s set.

I think from that perspective, we can think of what you’re calling the “workspace context” and “workspace group context” as essentially what, in the extension system, is just “state” and similar to how an extension gets its state both from the slot and from any additional properties defined in its declaration, we could effectively do something similar here. The “workspace container” gets a set of state (which can be changed when we change between different “containers”, like change between different slots).

I don’t think workspace groups map that cleanly to extensions (same with workspace group context mapping to extension states). With extension, we can just change its state and it immediately re-renders accordingly. However, with workspace group context, when we switch it, we need to prompt for unsaved changes, and the switching can fail if the user clicks cancel on the prompt. I think it should be a launch function call like the way it is now.

I’m still inclined to move towards removing the need for <WorkspaceContainer>.

I’d want to move that to another discussion. It’s a change that breaks a lot of downstream code and I don’t think it’s easy to test well.

I am, however, ok with moving the contextKey (effectively) into something of the workspace state. I think that an “proper support for restoring minimized workspaces” are the parts of this proposal I am enthusiastic about.

I don’t think lessening the distinction between workspace and workspace type is the right approach. What you’ve really done there is presented an argument for making “child workspaces” more of a thing, but to do that we need to know which workspaces belong together, and something like workspace type (maybe under a different name) is a good way of doing that especially as for a use-case like the order basket, we actually want multiple apps to contribute to what feels like a unified feature, at least in my view.

I’m also not sold on the proposal to only show the workspace action menu when launched. I actually think we need the side rail in more places.

I’m also not sold on the proposal to only show the workspace action menu when launched. I actually think we need the side rail in more places.

I’m actually proposing that the side rail be openable independent of workspaces, by having launchWorkspaceGroup() render the side rail, and closeWorkspaceGroup() remove it. Currently, launchWorkspaceGroup() has no direct UI effect; it only places a restriction that all workspaces opened afterwards need to be from the specified workspace group.

For example, in the patient chart, where we always have the 4 action item icons visible, we’d do something like launchWorkspaceGroup("patient-chart", context: <visitUuid>). OTOH, in the ward app, where we only see the action item icons when we launch a workspace, we’d have the ward patient card’s onClick do something like launchWorkspaceGroup('ward-patient-workspace', context: <patientUuid>, workspaceToLaunch: "ward-patient-workspace")

I don’t think lessening the distinction between workspace and workspace type is the right approach. What you’ve really done there is presented an argument for making “child workspaces” more of a thing, but to do that we need to know which workspaces belong together, and something like workspace type (maybe under a different name) is a good way of doing that especially as for a use-case like the order basket, we actually want multiple apps to contribute to what feels like a unified feature, at least in my view.

I haven’t prototyped, but it feels possible to just have the order basket be one single workspace, and we use the extension system to bring together the various order forms (like lab order and drug order), and have them conditionally rendered. (After all, workspaces are extensions.) Admittedly, I’m drawn to this approach partly because I had a fault start trying to implement child workspaces that got out of hand, and keeping one workspace per action item icon is simpler to implement.

Still, I’m not totally against having child workspaces per se. If we are to implement it, I think child workspaces should be opened differently, and they should preserve the same context / state from inherited from their parent workspace. (This is doable if we add a launchChildWorkspace callback in DefaultWorkspaceProp.) If we do that, then should there be any meaningful restriction on what can be opened as a child workspace from a parent workspace? I lean towards no, and that leads me to think that “workspace type” is still not a useful thing to have. The action item icons, workspace props and contexts would only be tied to the root workspaces (i.e. workspaces with no parent).

Some more random thoughts:

  • Some workspaces are “group context insensitive”, while others are not. For example, in the patient chart, there is an expectation that the visit notes , clinical forms, order basket, and vitals workspaces all operate with the same visit context. However, the patient list and the start visit workspaces, do not. This complicates our launchWorkspace context checking. I think the patient-search workspace used in the ward, appointments and service queues apps falls into this category too.

  • What if in the future, we combine the appointments and service queues into a single “Outpatient” app? Do we need to define a patient-search workspace for adding appointment and another patient-search workspace for adding queue entries? (Admittedly, this is a bit contrived as we can combine both into a single patient search workspace where we can have both “add appointment” and “add patient to queue” actions)

  • Similarly, if we are to replace the hard-coded visit note form and order basket with form engine forms, how do we model that? Presumably, we’d be using the same workspace component to render both. Do we need to define 2 different workspaces (albeit with the same component) in routes.json for both to show up properly in the workspace action menu?

  • It’s a subtle bug, but in the patient chart, if you click the clinical forms action icon to open the workspace, the icon does not get highlighted properly; it only highlights when we go into one of the forms. This is because the main clinical forms workspace has a different “type” compared to the form entry workspace and the action icon (clinical-forms vs clinical-form). It’s fixable, but it adds to my aversion of “workspace type”.

Update: Had a meeting with @ibacher and @dennis earlier this week. Some points:

  • Workspace group icons: We feel comfortable with associating a workspace group to a list of workspace action icons. (i.e. when we do launchWorkspaceGroup(), we see the corresponding icons of the group).
  • Workspace context and Workspace Group context: We feel comfortable with contexts for workspace and workspace group, and that changing context for either would prompt for unsaved changes to affected opened workspaces.
    • Currently, we already pass in props for workspaces (where it’s called additionalProps) and workspace groups (where it’s called state). In a sense, contexts are just props with additional consequences when they are updated. We want to explore whether it makes sense to just combine those two, since we cannot think of an example of a prop that can be updated without needing to prompt for unsaved changes. (In the original proposal, contexts were string type; with this, it will be an object type, similar to a React prop.)
  • Child workspaces: We will keep the idea of child workspace, but have proper support for it, so that when we open a child workspace from an existing workspace, it is context preserving.
  • Workspace type: We will keep “workspace type”, i.e. the mapping of a workspace action icon to a list of workspaces, albeit with a different name. Originally, I was hoping to get rid of workspace type if we can just designate one “root workspace” to be associated with an icon. However, this makes an incorrect assumption that there is one “root workspace” that always gets opened first within the list of workspaces.
  • Confguration-based workspace group: We should use the configuration system, and the flexibility it provides, for defining workspace groups. (Currently, workspace groups are defined in routes.json.) Here’s a sample schema we came up with:
workspaceGroup: {
  // action icon button
  appointment: {
    icon: <some-icon>
    workspaces: [
      // workspaces that map to the button
      'patient-search-appointment', 'appointment-form'
    ]
  }
}