(fix) O3-2345: Fix medication order form styling at phone and near-tablet widths#3229
(fix) O3-2345: Fix medication order form styling at phone and near-tablet widths#3229praneeth622 wants to merge 6 commits into
Conversation
…blet widths The drug order form had two responsive styling bugs: 1. The patient info header banner rendered unconditionally, appearing on desktop and phone when it should only show on tablet. Added an isTablet guard, matching the pattern in add-test-order.component. 2. At phone width, form inputs appeared greyed out because InputWrapper only applied Carbon Layer level 1 on tablet. Since the SCSS applies a grey background via .omrs-breakpoint-lt-desktop (both tablet and phone), phone inputs at Layer 0 blended into the background. Changed InputWrapper and ControlledFieldInput to use isDesktop() so phone correctly gets elevated Layer and touch-friendly input sizing.
There was a problem hiding this comment.
Pull request overview
Fixes responsive styling issues in the medication order form across phone/tablet/desktop breakpoints, aligning behavior with existing workspace patterns and Carbon layering.
Changes:
- Updated responsive sizing and Carbon
Layerelevation logic to treat all non-desktop layouts (tablet + phone) consistently. - Conditionally rendered the patient info header only on tablet to avoid duplication on desktop.
- Corrected SCSS breakpoint comments to match the actual
.omrs-breakpoint-lt-desktopscope (tablet + phone).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/esm-patient-medications-app/src/add-drug-order/drug-order-form.scss |
Clarifies breakpoint intent in comments for styles applied under lt-desktop. |
packages/esm-patient-medications-app/src/add-drug-order/drug-order-form.component.tsx |
Adjusts layout-driven rendering/sizing (patient header visibility, input/date picker sizes, and Layer level) to resolve phone/tablet styling issues. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function InputWrapper({ children }) { | ||
| const isTablet = useLayoutType() === 'tablet'; | ||
| const layout = useLayoutType(); | ||
| return ( | ||
| <Layer level={isTablet ? 1 : 0}> | ||
| <Layer level={isDesktop(layout) ? 0 : 1}> | ||
| <div className={styles.field}>{children}</div> |
There was a problem hiding this comment.
There are now multiple layout-dependent branches (e.g., Layer level and input/date picker size switching based on isDesktop(layout)), but the existing drug-order-form.test.tsx suite doesn’t cover any useLayoutType-driven behavior. Please add a couple of tests that mock useLayoutType to at least assert the desktop vs non-desktop rendering differences (e.g., patient header presence on tablet only, and the non-desktop sizing/layer settings), to prevent regressions in responsive behavior.
Adds tests verifying that the patient header banner is only rendered on tablet layout and is hidden on desktop and phone viewports.
There was a problem hiding this comment.
Thanks for the well-documented PR, @praneeth622. A few things:
-
Could you post screenshots in the PR description.
-
The patient header guard (
{isTablet && (...)}) is a good fix — on desktop, the patient banner is already visible at the top of the chart, so rendering it again inside the workspace duplicates information. On tablet, the workspace takes over the full screen and the chart banner isn't visible, so the inline header is needed. This matches the existing pattern inadd-test-order.component.tsx. -
One thing to flag: O3 doesn't support phone-width viewports (< 601px). See https://zpl.io/YYyl3ze. That means Bug 2 (inputs greyed out at phone width) isn't a real user-facing issue.
-
The
isDesktop(layout) ? 0 : 1change is functionally harmless, but it diverges from theisTablet ? 1 : 0pattern used intest-order-form.component.tsx,general-order-form.component.tsx, andexported-lab-results-form.workspace.tsx. I'd prefer keeping the existingisTabletpattern for consistency, since phone-width behavior doesn't need to be correct.
The patient header fix is worth keeping. Could you revert the isDesktop changes and keep just the {isTablet && (...)} guard?
…nsistency Per review feedback: O3 does not support phone-width viewports, so the phone-specific styling is not a real user-facing concern. Revert the isDesktop(layout) ? X : Y changes back to the existing isTablet ? Y : X pattern to stay consistent with test-order-form.component.tsx, general-order-form.component.tsx, and exported-lab-results-form.workspace.tsx. Keeps the isTablet guard on the patient header banner (the main fix).
|
Thanks for the review, @denniskigen ! I’ve reverted the isDesktop changes and kept the isTablet pattern for consistency with the other form components, retaining only the {isTablet && (...)} patient header guard as the actual fix. Regarding the Zeplin link (https://zpl.io/YYyl3ze) — I don’t currently have access to that workspace. Could you please grant access to my account? My email is praneethdevarasetty31@gmail.com. Thanks! |
Requirements
Summary
Fixes O3-2345: The medication order form had two responsive styling bugs:
Bug 1 — Patient info banner visible outside tablet
The
patientHeader(patient name/age/gender bar) rendered unconditionally on all screen sizes. On desktop, this duplicates the patient banner already visible in the main chart. Added an{isTablet && (...)}guard, matching the pattern already used inadd-test-order.component.tsx.Bug 2 — Inputs greyed out at phone width
The SCSS applies a grey background (
#ededed) to the order form via.omrs-breakpoint-lt-desktop, which targets both tablet and phone. However,InputWrapperonly applied CarbonLayer level={1}on tablet — on phone,Layer level={0}caused inputs to blend into the grey background, appearing greyed out/disabled.Fixed by using
isDesktop()from@openmrs/esm-frameworkinstead of a bareisTabletcheck, so both tablet and phone get the elevated Layer and touch-friendly input sizing.Changes
InputWrapper: UseisDesktop(layout) ? 0 : 1for Layer level (phone now gets white input backgrounds)patientHeader: Only render on tablet ({isTablet && (...)})ControlledFieldInput: UseisDesktop(layout)for responsive sizing (phone getsmdinstead ofsm)OpenmrsDatePicker: UseisDesktop(layout)for size (phone getslginstead ofsm)/* Desktop */→/* Tablet and Phone */)Screenshots
Testing at different viewport widths:
Phone (< 600px): Inputs have white backgrounds (not greyed out). No patient header banner. Touch-friendly
md/lgsizing.Tablet (601-1023px): Patient header banner visible. Inputs white. CustomNumberInput with steppers for duration/refills.
Desktop (1024px+): No patient header banner. Compact
smsizing. Standard NumberInput.Related Issue
https://openmrs.atlassian.net/browse/O3-2345
Other
N/A