Skip to content

Fix: Move action buttons to sticky footer in AddChargeItemsBillingSheet #16099 #16118

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.

Sign up for GitHub

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

Closed
niyati10000 wants to merge 1 commit into from
+25 −26
Closed

Fix: Move action buttons to sticky footer in AddChargeItemsBillingSheet #16099 #16118

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 25 additions & 26 deletions src/pages/Facility/billing/account/components/AddChargeItemsBillingSheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { ScrollArea } from "@/components/ui/scroll-area";
import {
Sheet,
SheetContent,
SheetFooter,
SheetHeader,
SheetTitle,
} from "@/components/ui/sheet";
Expand Down Expand Up @@ -159,7 +160,7 @@ export default function AddChargeItemsBillingSheet({
<SheetHeader className="px-4 py-4">
<SheetTitle>{t("add_charge_items")}</SheetTitle>
</SheetHeader>
<ScrollArea className="flex-1 pb-12 px-4 pt-0">
<ScrollArea className="flex-1 px-4 pt-0">
<div className="mt-4 space-y-4">
{selectedItems.length > 0 && (
<div className="space-y-2">
Expand All @@ -171,7 +172,6 @@ export default function AddChargeItemsBillingSheet({
key={index}
className="bg-white rounded-lg border p-4 space-y-3"
>
{/* Title and Remove Button */}
<div className="flex items-start justify-between gap-2">
<h4 className="font-medium text-base flex-1">
{item.charge_item_definition_object.title}
Expand All @@ -186,7 +186,6 @@ export default function AddChargeItemsBillingSheet({
</Button>
</div>

{/* Quantity and Price */}
<div className="flex flex-wrap gap-4 items-center">
<div className="space-y-1">
<label className="text-sm text-gray-500">
Expand Down Expand Up @@ -373,31 +372,31 @@ export default function AddChargeItemsBillingSheet({
defaultOpen={open}
/>
</div>

<div className="flex justify-end gap-2">
<Button
variant="outline"
onClick={() => onOpenChange(false)}
disabled={isPending}
>
{t("cancel")}
<ShortcutBadge actionId="cancel-action" />
</Button>
<Button
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
handleSubmit();
}}
disabled={isPending || selectedItems.length === 0 || disabled}
className="flex flex-row items-center gap-2 justify-between"
>
{t("add_items")}
{open && <ShortcutBadge actionId="enter-action" />}
</Button>
</div>
</div>
</ScrollArea>
{/* Fixed Footer with Action Buttons */}
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
Copy link

@greptile-apps greptile-apps bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double spacing on sm screens from gap-2 + sm:space-x-2

SheetFooter applies sm:space-x-2 by default (via cn("flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2", className) in sheet.tsx). Since tailwind-merge does not treat gap-* and space-x-* as conflicting utilities (they produce different CSS properties), both will be applied simultaneously on sm and larger breakpoints — resulting in ~8px of margin-left from space-x-2 stacked on top of the 8px gap-2, giving roughly 16px of total spacing between the two buttons instead of the intended 8px.

To fix this, either negate the default sm:space-x-2 or don't add gap-2 here and instead rely on the default:

Suggested change
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2 sm:space-x-0">

Copy link

@greptile-apps greptile-apps bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sticky bottom-0 is redundant in this layout

SheetContent is already a fixed-position element with flex flex-col and h-full (from inset-y-0). ScrollArea has flex-1, so it consumes all remaining vertical space — the SheetFooter naturally sits at the bottom of the visible panel as the next flex sibling, with no ability to scroll past it. The sticky bottom-0 class has no practical effect here and may be misleading.

Consider removing sticky bottom-0 to keep the intent clear:

Suggested change
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
<SheetFooter className="p-4 border-t bg-white flex-row justify-end gap-2 sm:space-x-0">

Copy link

@greptile-apps greptile-apps bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing dark mode background on footer

bg-white is hardcoded without a dark-mode counterpart. The SheetContent base styles include dark:bg-gray-950, so on dark mode the footer background will visually mismatch the panel. Consider pairing it with a dark mode class:

Suggested change
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
<SheetFooter className="p-4 border-t bg-white dark:bg-gray-950 sticky bottom-0 flex-row justify-end gap-2 sm:space-x-0">

Copy link
Contributor

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid overriding footer responsiveness and hardcoded background on Line 378.

flex-row overrides SheetFooter’s mobile-friendly default stacking, and bg-white can break theme/high-contrast behavior. Keep responsive defaults and use semantic background tokens.

Proposed fix 
-        <SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
+        <SheetFooter className="sticky bottom-0 border-t bg-background p-4 gap-2">

As per coding guidelines, "Use Tailwind's responsive classes and follow mobile-first approach" and "Include high contrast support for visibility in various clinical lighting conditions."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<SheetFooter className="p-4 border-t bg-white sticky bottom-0 flex-row justify-end gap-2">
<SheetFooter className="sticky bottom-0 border-t bg-background p-4 gap-2">
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/pages/Facility/billing/account/components/AddChargeItemsBillingSheet.tsx`
at line 378, The SheetFooter’s className is forcing desktop-only layout and a
hardcoded color; remove the literal "flex-row" and "bg-white" from the
SheetFooter className in AddChargeItemsBillingSheet so the component retains its
mobile-first stacked behavior and theme/contrast support, replace "flex-row"
with a responsive class like "sm:flex-row" (or "md:flex-row" if preferred) to
enable horizontal layout only on larger screens, and swap "bg-white" for a
semantic background token used in the app (e.g., "bg-surface" or
"bg-background") to preserve theme and high-contrast modes.

<Button
variant="outline"
onClick={() => onOpenChange(false)}
disabled={isPending}
>
{t("cancel")}
<ShortcutBadge actionId="cancel-action" />
</Button>
<Button
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
handleSubmit();
}}
disabled={isPending || selectedItems.length === 0 || disabled}
className="flex flex-row items-center gap-2"
>
{t("add_items")}
{open && <ShortcutBadge actionId="enter-action" />}
</Button>
</SheetFooter>
</SheetContent>
</Sheet>
);
Expand Down
Loading