-
Notifications
You must be signed in to change notification settings - Fork 335
fix(mobile): improve native app feel on mobile browsers #121
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
Open
qard-spacebot
wants to merge
8
commits into
spacedriveapp:main
Choose a base branch
from
qard-spacebot:mobile-tab-scroll
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f81e02d
feat(ui): make AgentTabs horizontally scrollable on mobile
cdea999
fix: prevent sidebar from being excluded from flex layout
c76ac57
fix: prevent iOS auto-zoom on input focus at mobile viewport widths
c1ebf83
fix: use !important to ensure mobile input font-size overrides Tailwind
51a5549
Add SettingSectionNav shared component
1e90875
Use SettingSectionNav in Settings, add responsive wrapper
bc4f42f
Use SettingSectionNav in AgentConfig, add responsive wrapper
9065856
fix: address PR review comments
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import { render, screen } from "@testing-library/react"; | ||
| import { AgentTabs } from "./AgentTabs"; | ||
|
|
||
| vi.mock("@tanstack/react-router", () => ({ | ||
| useMatchRoute: () => () => false, | ||
| Link: ({ children, className }: { children: React.ReactNode; className?: string; to?: string; params?: unknown }) => ( | ||
| <a className={className}>{children}</a> | ||
| ), | ||
| })); | ||
|
|
||
| describe("AgentTabs", () => { | ||
| it("renders 11 tab links", () => { | ||
| const { container } = render(<AgentTabs agentId="main" />); | ||
| const links = container.querySelectorAll("a"); | ||
| expect(links).toHaveLength(11); | ||
| }); | ||
|
|
||
| it("container has flex and h-12 classes", () => { | ||
| const { container } = render(<AgentTabs agentId="main" />); | ||
| const div = container.firstElementChild!; | ||
| expect(div).toHaveClass("flex"); | ||
| expect(div).toHaveClass("h-12"); | ||
| expect(div).toHaveClass("border-b"); | ||
| }); | ||
|
|
||
| it("container has overflow-x-auto class", () => { | ||
| const { container } = render(<AgentTabs agentId="main" />); | ||
| const div = container.firstElementChild!; | ||
| expect(div).toHaveClass("overflow-x-auto"); | ||
| }); | ||
|
|
||
| it("container has no-scrollbar class", () => { | ||
| const { container } = render(<AgentTabs agentId="main" />); | ||
| const div = container.firstElementChild!; | ||
| expect(div).toHaveClass("no-scrollbar"); | ||
| }); | ||
|
|
||
| it("renders all tab labels", () => { | ||
| render(<AgentTabs agentId="main" />); | ||
| expect(screen.getByText("Overview")).toBeDefined(); | ||
| expect(screen.getByText("Chat")).toBeDefined(); | ||
| expect(screen.getByText("Config")).toBeDefined(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import { render } from "@testing-library/react"; | ||
| import { SettingSectionNav } from "./SettingSectionNav"; | ||
|
|
||
| const singleGroup = [ | ||
| { | ||
| label: "Settings", | ||
| sections: [ | ||
| { id: "a", label: "Section A" }, | ||
| { id: "b", label: "Section B" }, | ||
| { id: "c", label: "Section C" }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| const multiGroup = [ | ||
| { | ||
| label: "Group One", | ||
| sections: [ | ||
| { id: "a", label: "Section A" }, | ||
| { id: "b", label: "Section B" }, | ||
| ], | ||
| }, | ||
| { | ||
| label: "Group Two", | ||
| sections: [ | ||
| { id: "c", label: "Section C" }, | ||
| { id: "d", label: "Section D" }, | ||
| { id: "e", label: "Section E" }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| describe("SettingSectionNav", () => { | ||
| it("desktop sidebar div has hidden md:flex classes", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={singleGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const desktopDiv = container.children[0] as HTMLElement; | ||
| expect(desktopDiv).toHaveClass("hidden"); | ||
| expect(desktopDiv).toHaveClass("md:flex"); | ||
| }); | ||
|
|
||
| it("mobile tab bar div has flex md:hidden classes", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={singleGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| expect(mobileDiv).toHaveClass("flex"); | ||
| expect(mobileDiv).toHaveClass("md:hidden"); | ||
| }); | ||
|
|
||
| it("renders correct number of buttons for a single-group config", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={singleGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const desktopDiv = container.children[0] as HTMLElement; | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| expect(desktopDiv.querySelectorAll("button")).toHaveLength(3); | ||
| expect(mobileDiv.querySelectorAll("button")).toHaveLength(3); | ||
| }); | ||
|
|
||
| it("renders correct number of buttons for a multi-group config", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={multiGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const desktopDiv = container.children[0] as HTMLElement; | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| expect(desktopDiv.querySelectorAll("button")).toHaveLength(5); | ||
| expect(mobileDiv.querySelectorAll("button")).toHaveLength(5); | ||
| }); | ||
|
|
||
| it("group separator rendered in mobile bar when multiple groups provided", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={multiGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| const separators = mobileDiv.querySelectorAll("div.w-px"); | ||
| expect(separators).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("no separator rendered in mobile bar for single group", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={singleGroup} activeSection="a" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| const separators = mobileDiv.querySelectorAll("div.w-px"); | ||
| expect(separators).toHaveLength(0); | ||
| }); | ||
|
|
||
| it("active section button has active styling class", () => { | ||
| const { container } = render( | ||
| <SettingSectionNav groups={singleGroup} activeSection="b" onSectionChange={vi.fn()} />, | ||
| ); | ||
| const desktopDiv = container.children[0] as HTMLElement; | ||
| const desktopButtons = desktopDiv.querySelectorAll("button"); | ||
| expect(desktopButtons[1]).toHaveClass("bg-app-darkBox"); | ||
| const mobileDiv = container.children[1] as HTMLElement; | ||
| const mobileButtons = mobileDiv.querySelectorAll("button"); | ||
| expect(mobileButtons[1]).toHaveClass("text-ink"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import React from "react"; | ||
| import {SettingSidebarButton} from "@/ui"; | ||
|
|
||
| interface SectionGroup { | ||
| label: string; | ||
| sections: { | ||
| id: string; | ||
| label: string; | ||
| badge?: React.ReactNode; | ||
| }[]; | ||
| } | ||
|
|
||
| interface SettingSectionNavProps { | ||
| groups: SectionGroup[]; | ||
| activeSection: string; | ||
| onSectionChange: (id: string) => void; | ||
| } | ||
|
|
||
| export function SettingSectionNav({groups, activeSection, onSectionChange}: SettingSectionNavProps) { | ||
| return ( | ||
| <> | ||
| {/* Desktop sidebar */} | ||
| <div className="hidden md:flex w-52 flex-shrink-0 flex-col border-r border-app-line/50 bg-app-darkBox/20 overflow-y-auto"> | ||
| {groups.map((group) => ( | ||
| <React.Fragment key={group.label}> | ||
| <div className="px-3 pb-1 pt-4"> | ||
| <span className="text-tiny font-medium uppercase tracking-wider text-ink-faint"> | ||
| {group.label} | ||
| </span> | ||
| </div> | ||
| <div className="flex flex-col gap-0.5 px-2"> | ||
| {group.sections.map((section) => ( | ||
| <SettingSidebarButton | ||
| key={section.id} | ||
| type="button" | ||
| onClick={() => onSectionChange(section.id)} | ||
| active={activeSection === section.id} | ||
| > | ||
| <span className="flex-1">{section.label}</span> | ||
| {section.badge} | ||
| </SettingSidebarButton> | ||
| ))} | ||
| </div> | ||
| </React.Fragment> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* Mobile tab bar */} | ||
| <div className="flex md:hidden h-10 items-stretch overflow-x-auto no-scrollbar border-b border-app-line/50 bg-app-darkBox/20 px-2"> | ||
| {groups.map((group, groupIndex) => ( | ||
| <React.Fragment key={group.label}> | ||
| {groupIndex > 0 && ( | ||
| <div className="w-px bg-app-line/30 self-stretch my-1" /> | ||
| )} | ||
| {group.sections.map((section) => { | ||
| const isActive = activeSection === section.id; | ||
| return ( | ||
| <button | ||
| key={section.id} | ||
| type="button" | ||
| onClick={() => onSectionChange(section.id)} | ||
| className={`relative whitespace-nowrap px-3 text-sm transition-colors ${ | ||
| isActive ? "text-ink" : "text-ink-faint hover:text-ink-dull" | ||
| }`} | ||
| > | ||
| {section.label} | ||
| {isActive && ( | ||
| <div className="absolute bottom-0 left-0 right-0 h-px bg-accent" /> | ||
| )} | ||
| </button> | ||
| ); | ||
| })} | ||
| </React.Fragment> | ||
| ))} | ||
| </div> | ||
| </> | ||
| ); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.