Skip to content

fix(mobile): Clean up action buttons, layout, and remove duplicate sidebar toggle #1001

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

Merged
hotlong merged 4 commits into from
Mar 3, 2026
Merged

fix(mobile): Clean up action buttons, layout, and remove duplicate sidebar toggle #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a689b44
Initial plan
Copilot Mar 3, 2026
893e11b
fix(mobile): clean up action buttons, layout, and remove duplicate si…
Copilot Mar 3, 2026
5ee15a8
fix: address code review feedback - remove redundant sm: prefix, impr…
Copilot Mar 3, 2026
5ac3109
fix(DetailSection): use responsive breakpoints for grid columns inste…
Copilot Mar 3, 2026
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
9 changes: 8 additions & 1 deletion packages/components/src/renderers/action/action-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { ComponentRegistry } from '@object-ui/core';
import type { ActionSchema, ActionLocation, ActionComponent } from '@object-ui/types';
import { useCondition } from '@object-ui/react';
import { cn } from '../../lib/utils';
import { useIsMobile } from '../../hooks/use-mobile';

export interface ActionBarSchema {
type: 'action:bar';
Expand All @@ -45,6 +46,8 @@ export interface ActionBarSchema {
location?: ActionLocation;
/** Maximum visible inline actions before overflow into "More" menu (default: 3) */
maxVisible?: number;
/** Maximum visible inline actions on mobile devices (default: 1). Desktop uses maxVisible instead. */
mobileMaxVisible?: number;
/** Visibility condition expression */
visible?: string;
/** Layout direction */
Expand All @@ -71,6 +74,7 @@ const ActionBarRenderer = forwardRef<HTMLDivElement, { schema: ActionBarSchema;
} = props;

const isVisible = useCondition(schema.visible ? `\${${schema.visible}}` : undefined);
const isMobile = useIsMobile();

// Filter actions by location
const filteredActions = useMemo(() => {
Expand All @@ -82,7 +86,10 @@ const ActionBarRenderer = forwardRef<HTMLDivElement, { schema: ActionBarSchema;
}, [schema.actions, schema.location]);

// Split into visible inline actions and overflow
const maxVisible = schema.maxVisible ?? 3;
// On mobile, show fewer actions inline (default: 1)
const maxVisible = isMobile
? (schema.mobileMaxVisible ?? 1)
: (schema.maxVisible ?? 3);
Comment on lines 76 to +92
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

useIsMobile() relies on window.matchMedia() (and addEventListener) without a guard. Now that ActionBar always calls it, environments that don't provide matchMedia (unit tests, some non-browser runtimes) will throw during effect execution. Either make useIsMobile resilient when window.matchMedia is missing, or provide a global matchMedia mock in the test setup so ActionBar rendering can't crash.

Copilot uses AI. Check for mistakes.
const { inlineActions, overflowActions } = useMemo(() => {
if (filteredActions.length <= maxVisible) {
return { inlineActions: filteredActions, overflowActions: [] as ActionSchema[] };
Expand Down
4 changes: 2 additions & 2 deletions packages/layout/src/AppShell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ export function AppShell({
{sidebar}
<SidebarInset>
<header className="flex h-14 sm:h-16 shrink-0 items-center gap-2 border-b bg-background px-2 sm:px-4">
<SidebarTrigger className="-ml-1" />
<div className="w-px h-4 bg-border mx-1 sm:mx-2" />
<SidebarTrigger className="-ml-1 hidden md:inline-flex" />
<div className="w-px h-4 bg-border mx-1 sm:mx-2 hidden md:block" />
Comment on lines +139 to +140
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Hiding the only built-in below md makes the sidebar impossible to open on mobile for AppShell consumers that do not render their own mobile trigger in navbar (e.g. AppSchemaRenderer when navbar is undefined, and AppShell Storybook stories). Consider gating this behind an explicit AppShell prop (e.g. hideTriggerOnMobile) or keeping the trigger visible by default and letting apps opt into hiding it when they provide their own header trigger.

Suggested change
<SidebarTrigger className="-ml-1 hidden md:inline-flex" />
<div className="w-px h-4 bg-border mx-1 sm:mx-2 hidden md:block" />
<SidebarTrigger className="-ml-1 inline-flex" />
<div className="w-px h-4 bg-border mx-1 sm:mx-2 block" />

Copilot uses AI. Check for mistakes.
{navbar}
</header>
<main className={cn("flex-1 min-w-0 overflow-auto p-3 sm:p-4 md:p-6", className)}>
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-detail/src/DetailSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
className={cn(
"grid gap-3 sm:gap-4",
effectiveColumns === 1 ? "grid-cols-1" :
effectiveColumns === 2 ? "grid-cols-1 md:grid-cols-2" :
effectiveColumns === 3 ? "grid-cols-1 md:grid-cols-2 lg:grid-cols-3" :
"grid-cols-1 md:grid-cols-2 lg:grid-cols-3"
effectiveColumns === 2 ? "grid-cols-1 sm:grid-cols-2" :
effectiveColumns === 3 ? "grid-cols-1 sm:grid-cols-2 lg:grid-cols-3" :
"grid-cols-1 sm:grid-cols-2 lg:grid-cols-3"
Comment on lines +198 to +200
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This breakpoint change (md→sm) will break existing unit tests that assert md:grid-cols-2 / md:grid-cols-2 lg:grid-cols-3 for auto-layout. Update DetailSection.test.tsx expectations to match the new responsive classes (sm:grid-cols-2, etc.) so CI reflects the intended behavior.

Copilot uses AI. Check for mistakes.
)}
>
{layoutFields.map(renderField)}
Expand Down
30 changes: 24 additions & 6 deletions packages/plugin-detail/src/DetailView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,15 @@ export const DetailView: React.FC<DetailViewProps> = ({
<SchemaRenderer key={index} schema={action} data={data} />
))}

{/* Inline Edit Toggle */}
{/* Inline Edit Toggle - hidden on mobile, accessible via more menu */}
{inlineEdit && (
<Tooltip>
<TooltipTrigger asChild>
<Button
variant={isInlineEditing ? 'default' : 'outline'}
size="sm"
onClick={handleInlineEditToggle}
className="gap-2"
className="gap-2 hidden sm:inline-flex"
>
{isInlineEditing ? (
<>
Expand All @@ -498,21 +498,21 @@ export const DetailView: React.FC<DetailViewProps> = ({
</Tooltip>
)}

{/* Share Button */}
{/* Share Button - hidden on mobile, accessible via more menu */}
<Tooltip>
<TooltipTrigger asChild>
<Button variant="outline" size="icon" onClick={handleShare}>
<Button variant="outline" size="icon" onClick={handleShare} className="hidden sm:inline-flex">
<Share2 className="h-4 w-4" />
</Button>
</TooltipTrigger>
<TooltipContent>{t('detail.share')}</TooltipContent>
</Tooltip>

{/* Edit Button */}
{/* Edit Button - hidden on mobile, accessible via more menu */}
{schema.showEdit && (
<Tooltip>
<TooltipTrigger asChild>
<Button variant="default" onClick={handleEdit} className="gap-2">
<Button variant="default" onClick={handleEdit} className="gap-2 hidden sm:inline-flex">
<Edit className="h-4 w-4" />
<span className="hidden sm:inline">{t('detail.edit')}</span>
</Button>
Expand All @@ -534,6 +534,24 @@ export const DetailView: React.FC<DetailViewProps> = ({
<TooltipContent>{t('detail.moreActions')}</TooltipContent>
</Tooltip>
<DropdownMenuContent align="end" className="w-[calc(100vw-2rem)] sm:w-48 max-h-[60vh] overflow-y-auto">
{/* Mobile-only: Share, Edit, Inline Edit */}
<DropdownMenuItem onClick={handleShare} className="sm:hidden">
<Share2 className="h-4 w-4 mr-2" />
{t('detail.share')}
</DropdownMenuItem>
{schema.showEdit && (
<DropdownMenuItem onClick={handleEdit} className="sm:hidden">
<Edit className="h-4 w-4 mr-2" />
{t('detail.edit')}
</DropdownMenuItem>
)}
{inlineEdit && (
<DropdownMenuItem onClick={handleInlineEditToggle} className="sm:hidden">
<Edit className="h-4 w-4 mr-2" />
{isInlineEditing ? t('detail.save') : t('detail.editInline')}
</DropdownMenuItem>
)}
<DropdownMenuSeparator className="sm:hidden" />
<DropdownMenuItem onClick={handleDuplicate}>
<Copy className="h-4 w-4 mr-2" />
{t('detail.duplicate')}
Expand Down