(fix) O3-4116 use correct colours for service queue priority

[flosell]

Requirements

• This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
• My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
• My work includes tests or is validated by existing tests.

Summary

(First PR here so please let me know if I'm doing something wrong)

O3-4116 notes that priority tags in the service queue view don't have any color.

Implementers were already able to configure this, however this was probably not used frequently. So I'm guessing the intended behaviour is a default configuration that can be overwritten if necessary. This is what this PR adds.

The PR also adds support for the orange color intended for the 'urgent' priority that was still missing.

Screenshots

Related Issue

https://openmrs.atlassian.net/browse/O3-4116

Other

I'm not very happy the way the orange color is handled: Seems like the carbon design system does not have a color 'orange' so I implemented it as a special case similar to how the tags for the header in the patient-chart are implemented. I'm also hardcoding the same color. This seems to be slightly smelly. I'm guessing this component in this color is used in a few different places so there should be a generic solution for this problem but I'm not sure how to approach it... Suggestions welcome!

Also, the issue also asks for fixing capitalization for 'Not Urgent' - I haven't looked into this yet so might be another PR

[chibongho]

I'm guessing this component in this color is used in a few different places so there should be a generic solution for this problem but I'm not sure how to approach it... Suggestions welcome!

Yeah, the patient chart ought to be using the same component as the services queues app to render those priority tags. It's usually done through extensions (https://o3-docs.openmrs.org/docs/extension-system). I suggest having the PriorityTag be its own component so it's closer to what we want.

Also, the issue also asks for fixing capitalization for 'Not Urgent' - I haven't looked into this yet so might be another PR
I think the string is defined here (I don't have access to change it). https://app.openconceptlab.org/#/orgs/openmrs/sources/DemoQueueConcepts/concepts/DemoQueue-Status/

[flosell]

Yeah, the patient chart ought to be using the same component as the services queues app to render those priority tags. It's usually done through extensions (https://o3-docs.openmrs.org/docs/extension-system). I suggest having the PriorityTag be its own component so it's closer to what we want.

Hi @chibongho - thanks for the input! I thought the extension system is more for cases where we might want to swap out or swap in various extensions into a slot.
In this case, it's a fairly well-defined component that would be plugged into the services queues and the patient chart. So might this be more a case similar to patient-photo, a simple react component that can be used in multiple places?

I also looked at the patient chart implementation a bit more, seems there's also a bit different behavior, e.g. what's 'Urgent' on the service queue is 'Priority' in the patient chart.
All in all, sounds like a bit of a bigger change - should that refactoring (and aligning the behavior) be tackled in a separate issue?

[chibongho]

I thought the extension system is more for cases where we might want to swap out or swap in various extensions into a slot.

Yeah, you're right. (In fact, the extension system has often been misused for other purposes.) I would not be opposed to putting the priority tag in a common library. I guess one reason for using an extension is that it is possible for certain implementation to have the patient chart but not service queues functionality (although the current patient chart code isn't respecting that either.)

As for which common library to put it in, esm-styleguide historically has a fairly high bar for admitting components. esm-patient-common-lib was originally meant only for packages within openmrs-esm-patient-chart, but is now depended on by openmrs-esm-patient-management as well. That seems like a good place to put it. What do you think @denniskigen @ibacher ?

I also looked at the patient chart implementation a bit more, seems there's also a bit different behavior, e.g. what's 'Urgent' on the service queue is 'Priority' in the patient chart. All in all, sounds like a bit of a bigger change - should that refactoring (and aligning the behavior) be tackled in a separate issue?

TBH, the behavior in patient chart is probably wrong. But yeah, let's not worry about that for now. I was suggesting that we have the priority tag be its own component, mostly to avoid the big <Tag> component being there twice.

[ibacher]

I’d prefer to do things in the styleguide than in the patient-common-lib. As much as possible, we want to avoid adding things there that are used outside the patient chart (and we’ve been trying to remove the stuff that makes things like patient management depend on it).

It’s not clear to me what this would add as a generic component over and above the Carbon Tag element (in particular, things that need configuration are a poor fit for the styleguide or the patient common lib).

[flosell]

@ibacher - I guess the main addition on top of the Carbon Tag element would be the orange color for Priority / Urgent.
But I sense maybe it's not enough to warrant a larger refactoring...

@chibongho I've pulled the tag into a variable to avoid the duplication - if we pulled it out into its own component there isn't really much left in QueuePriority so I kept the two things together

[ibacher]

fixing capitalization

The capitalization here is a non-code issue, so I wouldn't worry about it. Fixing capitalization in code always interferes with i18n.

[flosell]

Hi! Any more feedback? I'll be on vacation from the end of the week so any later feedback I won't be able to work on for a few weeks.