-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add tablet responsive support #108
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
base: main
Are you sure you want to change the base?
Changes from all commits
a862c2c
5051ea4
a4de3d2
558cf82
326891a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,7 @@ import { SettingsDialog } from '@brainbox/ui/components/settings/settings-dialog | |||||||||||||||||||||||||
| import { LayoutContext } from '@brainbox/ui/contexts/layout'; | ||||||||||||||||||||||||||
| import { useServer } from '@brainbox/ui/contexts/server'; | ||||||||||||||||||||||||||
| import { useWorkspace } from '@brainbox/ui/contexts/workspace'; | ||||||||||||||||||||||||||
| import { useDevice } from '@brainbox/ui/hooks/use-device'; | ||||||||||||||||||||||||||
| import { useLayoutState } from '@brainbox/ui/hooks/use-layout-state'; | ||||||||||||||||||||||||||
| import { useLiveQuery } from '@brainbox/ui/hooks/use-live-query'; | ||||||||||||||||||||||||||
| import { useWindowSize } from '@brainbox/ui/hooks/use-window-size'; | ||||||||||||||||||||||||||
|
|
@@ -24,6 +25,8 @@ export const Layout = () => { | |||||||||||||||||||||||||
| const server = useServer(); | ||||||||||||||||||||||||||
| const workspace = useWorkspace(); | ||||||||||||||||||||||||||
| const windowSize = useWindowSize(); | ||||||||||||||||||||||||||
| const { isTablet, isTouch } = useDevice(); | ||||||||||||||||||||||||||
| const useTouchHandles = isTablet || isTouch; | ||||||||||||||||||||||||||
| const [showShortcuts, setShowShortcuts] = useState(false); | ||||||||||||||||||||||||||
| const [showSearch, setShowSearch] = useState(false); | ||||||||||||||||||||||||||
| const [showSettings, setShowSettings] = useState(false); | ||||||||||||||||||||||||||
|
|
@@ -69,7 +72,7 @@ export const Layout = () => { | |||||||||||||||||||||||||
| !server.isOutdated && leftContainerMetadata.tabs.length > 0; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const shouldDisplayRight = | ||||||||||||||||||||||||||
| !server.isOutdated && rightContainerMetadata.tabs.length > 0; | ||||||||||||||||||||||||||
| !server.isOutdated && rightContainerMetadata.tabs.length > 0 && !isTablet; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||
| const handleKeyDown = (e: KeyboardEvent) => { | ||||||||||||||||||||||||||
|
|
@@ -91,6 +94,12 @@ export const Layout = () => { | |||||||||||||||||||||||||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||||||||||||||||||||||||||
| }, [handleSidebarToggle]); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||
| if (isTablet && !sidebarMetadata.collapsed) { | ||||||||||||||||||||||||||
| handleSidebarToggle(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }, [isTablet]); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| }, [isTablet]); | |
| }, [isTablet, sidebarMetadata.collapsed, handleSidebarToggle]); |
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.
logic: missing handleSidebarToggle in dependency array will cause stale closure
| useEffect(() => { | |
| if (isTablet && !sidebarMetadata.collapsed) { | |
| handleSidebarToggle(); | |
| } | |
| }, [isTablet]); | |
| useEffect(() => { | |
| if (isTablet && !sidebarMetadata.collapsed) { | |
| handleSidebarToggle(); | |
| } | |
| }, [isTablet, handleSidebarToggle, sidebarMetadata.collapsed]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/components/layouts/layout.tsx
Line: 97:101
Comment:
**logic:** missing `handleSidebarToggle` in dependency array will cause stale closure
```suggestion
useEffect(() => {
if (isTablet && !sidebarMetadata.collapsed) {
handleSidebarToggle();
}
}, [isTablet, handleSidebarToggle, sidebarMetadata.collapsed]);
```
How can I resolve this? If you propose a fix, please make it concise.
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.
The phone detection logic removed "iPad" from the regex, but modern iPads (iPadOS 13+) report their user agent as "Macintosh" when requesting desktop sites, making them indistinguishable from actual desktops via user agent alone. While this change allows tablets through, it may inadvertently allow iPads that could benefit from tablet-specific UX. Consider adding detection for iPads using feature detection (e.g., checking for touch support combined with large screen size) or relying solely on the viewport-based breakpoint detection in the
useDevicehook.