fix(ui): prevent duplicate scrollbars on Windows/Tauri #941
Conversation
github-actions
bot
added
UI
bug
📝 WalkthroughWalkthroughRemoves the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
frontend/src/utils/platform.ts (1)
13-15: Consider using @tauri-apps/api modules instead of relying on the injected global.The implementation is SSR-safe and functionally correct, but official Tauri documentation recommends importing from
@tauri-apps/apimodules rather than detecting the globalwindow.__TAURI__. The global approach only works when the build optionwithGlobalTauriis enabled. If platform detection via global is necessary, usetypeof window.__TAURI__ !== 'undefined'per the official pattern. For better maintainability and alignment with recommended practices, consider refactoring to use the @tauri-apps/api modules for feature detection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/layout/layout.tsxfrontend/src/pages/AITagging/AITagging.tsxfrontend/src/pages/Home/Home.tsxfrontend/src/pages/Home/MyFav.tsxfrontend/src/utils/platform.tslanding-page/src/App.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/pages/AITagging/AITagging.tsx (1)
frontend/src/utils/platform.ts (1)
isTauri(13-15)
frontend/src/pages/Home/MyFav.tsx (1)
frontend/src/utils/platform.ts (1)
isTauri(13-15)
frontend/src/pages/Home/Home.tsx (1)
frontend/src/utils/platform.ts (1)
isTauri(13-15)
landing-page/src/App.tsx (3)
frontend/src/pages/Home/Home.tsx (1)
Home(18-84)frontend/src/utils/platform.ts (1)
isTauri(13-15)landing-page/src/components/ui/ScrollProgress.tsx (1)
ScrollProgress(3-27)
🔇 Additional comments (5)
frontend/src/layout/layout.tsx (1)
23-23: LGTM - padding adjustment supports scrollbar positioning.Changing from margin to padding ensures the scrollbar is positioned within the container's bounds, which aligns with the PR objective to prevent clipping of native scrollbars on desktop platforms.
frontend/src/pages/Home/MyFav.tsx (1)
17-17: Import looks correct.The import statement properly references the new platform utility.
frontend/src/pages/Home/Home.tsx (1)
16-16: Import looks correct.The import statement properly references the new platform utility.
landing-page/src/App.tsx (1)
20-27: Verify scope: Expanded HomePage rendering appears unrelated to scrollbar fix.The HomePage component now renders additional sections (
PictopyLanding,FAQ,Footer) that were not previously included (lines 25-27). While the AI summary mentions this expansion, these additions don't appear directly related to fixing the duplicate scrollbar issue described in the PR objectives.If these additions are intentional refactoring or enhancement work, consider whether they should be in a separate PR for clearer change tracking.
frontend/src/pages/AITagging/AITagging.tsx (1)
16-16: LGTM: Import statement is correct.The import path and function name are valid.
| </div> | ||
| </div> | ||
| {monthMarkers.length > 0 && ( | ||
| {!isTauri && monthMarkers.length > 0 && ( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the AITagging.tsx file around line 73
head -80 frontend/src/pages/AITagging/AITagging.tsx | tail -10Repository: AOSSIE-Org/PictoPy
Length of output: 329
🏁 Script executed:
# Find where isTauri is defined
rg -n "export const isTauri|function isTauri" frontend/src --type ts --type tsxRepository: AOSSIE-Org/PictoPy
Length of output: 89
🏁 Script executed:
# Check all usages of isTauri throughout the codebase
rg -n "isTauri" frontend/src --type ts --type tsx -B 1 -A 1Repository: AOSSIE-Org/PictoPy
Length of output: 89
🏁 Script executed:
# Find isTauri definition - try without type flags
rg -n "export const isTauri|function isTauri|const isTauri" frontend/srcRepository: AOSSIE-Org/PictoPy
Length of output: 222
🏁 Script executed:
# Find where isTauri is imported from or defined
rg -n "isTauri" frontend/src --max-count 50Repository: AOSSIE-Org/PictoPy
Length of output: 1379
🏁 Script executed:
# List files in frontend/src to understand structure
find frontend/src -type f \( -name "*.ts" -o -name "*.tsx" \) | head -30Repository: AOSSIE-Org/PictoPy
Length of output: 1443
🏁 Script executed:
# Check if App.tsx uses isTauri
rg -n "isTauri" frontend/src/App.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Read the isTauri function definition
cat -n frontend/src/utils/platform.tsRepository: AOSSIE-Org/PictoPy
Length of output: 759
Fix missing function calls to isTauri().
isTauri is a function that must be invoked. The current code uses !isTauri without parentheses, which negates the function reference (always falsy), causing the condition to always be false. This breaks the conditional rendering—TimelineScrollbar will never render on any platform.
Same bug exists in:
frontend/src/pages/Home/Home.tsx:75frontend/src/pages/Home/MyFav.tsx:105
Fix
- {!isTauri && monthMarkers.length > 0 && (
+ {!isTauri() && monthMarkers.length > 0 && (📝 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.
| {!isTauri && monthMarkers.length > 0 && ( | |
| {!isTauri() && monthMarkers.length > 0 && ( |
🤖 Prompt for AI Agents
In frontend/src/pages/AITagging/AITagging.tsx around line 73, the conditional
uses !isTauri (a function reference) instead of calling it; change the condition
to call the function (use !isTauri()) so the boolean result is negated and the
TimelineScrollbar can render; apply the same fix in
frontend/src/pages/Home/Home.tsx at line ~75 and
frontend/src/pages/Home/MyFav.tsx at line ~105 (replace !isTauri with
!isTauri()).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/pages/AITagging/AITagging.tsxfrontend/src/pages/Home/Home.tsxfrontend/src/pages/Home/MyFav.tsxlanding-page/src/App.tsxlanding-page/src/utils/platform.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- landing-page/src/App.tsx
- frontend/src/pages/Home/Home.tsx
- frontend/src/pages/Home/MyFav.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/AITagging/AITagging.tsx (2)
landing-page/src/utils/platform.ts (1)
isTauri(5-7)frontend/src/utils/platform.ts (1)
isTauri(13-15)
🔇 Additional comments (2)
frontend/src/pages/AITagging/AITagging.tsx (2)
16-16: LGTM!The import is correctly structured and aligns with the platform detection utilities introduced in this PR.
73-73: Previous critical issue resolved.The function is now correctly invoked with
!isTauri()(with parentheses), fixing the bug flagged in the previous review where the function reference was negated instead of its return value. The conditional logic now properly gates TimelineScrollbar rendering for non-Tauri environments.
landing-page/src/utils/platform.ts
Outdated
| export const isTauri = (): boolean => { | ||
| return typeof window !== 'undefined' && '__TAURI__' in window; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Tauri environment detection JavaScript __TAURI__ property
💡 Result:
Use the global window.TAURI object (when injected) — check it safely, e.g.:
-
Minimal check:
const isTauri = typeof window !== 'undefined' && 'TAURI' in window; -
Safer (avoid runtime exceptions):
const isTauri = typeof window !== 'undefined' && window.TAURI !== undefined;
Notes:
- The global is only available if your app/webview injects it (v1 used build.withGlobalTauri / app.withGlobalTauri; v2 behavior can differ). If you rely on imports (recommended for bundlers) use @tauri-apps/api modules instead. [1][2]
- There are known edge cases (e.g., remote Dev/localhost access or migration issues) where window.TAURI may be undefined even inside a Tauri webview — handle fallbacks accordingly. [2][3]
Sources:
- @tauri-apps/api (vanilla JS / window.TAURI note). [1]
- Tauri GitHub discussions (detecting Tauri & migration/remote-injection issues). [2][3]
Consider using @tauri-apps/api imports for environment detection instead of relying on global injection.
The __TAURI__ detection pattern is correct and your SSR-safe check is a valid minimal approach. However, Tauri's recommended best practice is to use imports from @tauri-apps/api modules rather than relying on the global window.__TAURI__ object, which may not be injected in all scenarios (e.g., migration issues, remote development). Additionally, edge cases exist where window.__TAURI__ can be undefined even inside a Tauri webview, so consider whether a safer fallback is needed for robustness.
🤖 Prompt for AI Agents
In landing-page/src/utils/platform.ts around lines 5 to 7, replace the fragile
global check for window.__TAURI__ with a runtime-safe check that uses the
official @tauri-apps/api when available: keep the SSR guard (typeof window !==
'undefined'), then attempt a dynamic import of the relevant @tauri-apps/api
module inside a try/catch (or use a cached promise) and return true if the
import succeeds or the API indicates a Tauri environment; fall back to the
original window.__TAURI__ boolean check only if the import is unavailable, and
ensure no static imports to @tauri-apps/api run during server-side rendering.
There was a problem hiding this comment.
Thanks for the suggestion!
This was an intentional choice: I used a synchronous, SSR-safe check (typeof window !== 'undefined' && '__TAURI__' in window) for UI-only branching.
Using @tauri-apps/api would require introducing async detection and refactoring multiple render paths, which felt unnecessary for the scope of this UI fix. The current approach keeps the logic simple and predictable while addressing the Windows/Tauri scrollbar issue.
Happy to revisit this if broader platform detection is needed in the future.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
As suggested by @rahulharpal1603 , I initially tried reusing However, While the |
skyforge-glitch
force-pushed
the
fix/windows-tauri-scrollbar
branch
from
6b2c37a to
7884ff1
Compare
I’ve cleaned up the PR and reduced it to a focused one-line layout fix addressing the scrollbar issue on Windows. The branch was rebased and force-pushed accordingly. Would appreciate another look when you have time. @rahulharpal1603 |
1 participant
Summary
This PR fixes a UI issue on Windows/Tauri where multiple vertical scrollbars appeared when the application window was maximized.
The issue was caused by nested scrollable containers combined with Windows’ always-visible native scrollbars and custom scroll UI components.
Changes Made
utils/platform.ts) to reliably detect the Tauri environmentWhy This Fix Works
overflow-y-autocontainers that caused duplicate scrollbarsScope
Screenshots / Visual Notes
Related Issue
Closes #938
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.