-
Notifications
You must be signed in to change notification settings - Fork 232
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
(fix) O3-4299: Missing Label in Dropdown Menu of Service Queues #1425
base: main
Are you sure you want to change the base?
(fix) O3-4299: Missing Label in Dropdown Menu of Service Queues #1425
Conversation
@harshthakkr update test also |
@Muppasanipraneeth I am working on it. |
One test is failing due to the new year (2025). @chibongho has created a PR to update the test. Their PR needs to be merged first to ensure all e2e tests pass. |
@@ -67,7 +67,7 @@ function ClinicMetrics() { | |||
itemToString={(item) => | |||
item ? `${item.display} ${item.location?.display ? `- ${item.location.display}` : ''}` : '' | |||
} | |||
label="" | |||
label={t('all', 'All')} | |||
onChange={handleServiceChange} |
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.
Hardcoding All
as the label for the default item in the dropdown is not the right approach. If the goal is to show a default value for the dropdown, we should use the initialSelectedItem prop. This allows us to display a default value in the dropdown.
To achieve, this we can modify the logic as follows:
const defaultServiceItem = {
display: `${t('all', 'All')}`,
uuid: '',
};
const serviceItems = [defaultServiceItem, ...(services ?? [])];
And then further down the line, we can use these values to set the initialSelectedItem
prop and the items
prop:
<Dropdown
initialSelectedItem={defaultServiceItem}
items={serviceItems}
label=""
// ... other props
/>
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.
Two nice, optional extra touches would be:
- Annotating
serviceItems
with the proper types. - Fixing the styling of the dropdown so it's vertical alignment matches that of the label.
Requirements
Summary
This PR addresses an issue where the second clinic metric in Service Queues displayed an empty string instead of the label 'All' when defaulting to show data for all. The label has been updated to display 'All' by default, ensuring clarity and consistency in the UI.
Screenshots
Before:
After:
Related Issue
O3-4299