Skip to content

Comments

fix(ui): Window corner issue on Windows platform#11

Open
DunHouGo wants to merge 4 commits intodannysmith:mainfrom
DunHouGo:window-style
Open

fix(ui): Window corner issue on Windows platform#11
DunHouGo wants to merge 4 commits intodannysmith:mainfrom
DunHouGo:window-style

Conversation

@DunHouGo
Copy link

@DunHouGo DunHouGo commented Dec 30, 2025

The default style works well on Mac, but there are strange flaws on Windows.

It looks like this:
issue-windows-cornor

I found that this can be fixed with CSS styles, specifically the background color of the body.

Fixed style:
fixed-window-cornor
fixed-pannel-window

Summary by CodeRabbit

  • New Features

    • Platform detection added to optimise app appearance on Windows and macOS.
    • Added a close button to the Preferences dialog header.
  • Style

    • Background colours and border-radius now adapt per operating system for improved visuals.
    • Dialog overlay corners adjusted to match platform conventions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 30, 2025

Walkthrough

Adds platform detection (Windows vs mac) and applies platform-specific styling via a data-platform attribute and a new CSS variable --body-bg; components conditionally adjust corner radius and dialogs now include a header close button.

Changes

Cohort / File(s) Summary
Gitignore configuration
\.gitignore``
Adds an ignore rule for the repository root img/ directory (+/img/).
App bootstrap & platform hook
\src/App.tsx``
Introduces useIsWindows() usage, sets data-platform on document.documentElement ('windows' or 'mac'), effect now depends on [isWindows].
Global / quick-pane styles
\src/App.css`, `src/quick-pane.css``
Introduces --body-bg CSS variable and per-platform assignments (Windows: theme(colors.background); Mac/default: transparent); body/root use var(--body-bg, transparent).
Quick pane UI adjustments
\src/components/quick-pane/QuickPaneApp.tsx``
Adds getPlatform / useIsWindows imports and clsx; conditionally applies/removes rounded-xl on Windows, centralises platform-driven styling.
Main window rounding control
\src/components/layout/MainWindow.tsx``
Adds useIsWindows, wraps main container to apply rounded corners only on non-Windows platforms via conditional className.
Preferences dialog header
\src/components/preferences/PreferencesDialog.tsx``
Adds XIcon and Button import, inserts a close button in the dialog header, sets showCloseButton={false} on DialogContent.
Dialog overlay styling
\src/components/ui/dialog.tsx``
Removes rounded-xl from DialogOverlay className (commented Windows rationale) while keeping animation/positioning classes.
Package manifest
\package.json``
Mentioned in manifest section (no API signature changes reported).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 In code I hop and softly tread,

data-platform crowns the head,
Corners shave on Windows ground,
Mac stays rounded—joy abound,
A tiny rabbit cheers: "Well spread!" 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: addressing UI corner rendering issues on Windows platform through CSS variable assignment and platform detection.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/App.tsx (1)

20-122: Avoid including isWindows in the dependency array for one-time initialisation.

The platform doesn't change at runtime, yet including isWindows in the dependency array (line 122) will cause the entire effect to re-run if the hook's return value changes for any reason. This effect performs one-time initialisation tasks (command system, language/menu setup, file cleanup, update checks) that should not be repeated. The platform attribute setting (lines 28-32) should also only execute once.

🔎 Recommended fix

Move the platform attribute logic outside the effect or use a separate effect without dependencies:

  // Initialize command system and cleanup on app startup
  useEffect(() => {
    logger.info('🚀 Frontend application starting up')
    initializeCommandSystem()
    logger.debug('Command system initialized')

-    // Set platform attribute, windows need a special background color to fix the transparent background
-    // but mac need transparent background to show the window rounded corner
-    // not sure on linux
-    if (isWindows) {
-      document.documentElement.setAttribute('data-platform', 'windows')
-    } else {
-      document.documentElement.setAttribute('data-platform', 'mac')
-    }
-
    // Initialize language based on saved preference or system locale
    const initLanguageAndMenu = async () => {
      try {
        // Load preferences to get saved language
        const result = await commands.loadPreferences()
        const savedLanguage =
          result.status === 'ok' ? result.data.language : null

        // Initialize language (will use system locale if no preference)
        await initializeLanguage(savedLanguage)

        // Build the application menu with the initialized language
        await buildAppMenu()
        logger.debug('Application menu built')
        setupMenuLanguageListener()
      } catch (error) {
        logger.warn('Failed to initialize language or menu', { error })
      }
    }

    initLanguageAndMenu()

    // Clean up old recovery files on startup
    cleanupOldFiles().catch(error => {
      logger.warn('Failed to cleanup old recovery files', { error })
    })

    // Example of logging with context
    logger.info('App environment', {
      isDev: import.meta.env.DEV,
      mode: import.meta.env.MODE,
    })

    // Auto-updater logic - check for updates 5 seconds after app loads
    const checkForUpdates = async () => {
      try {
        const update = await check()
        if (update) {
          logger.info(`Update available: ${update.version}`)

          // Show confirmation dialog
          const shouldUpdate = confirm(
            `Update available: ${update.version}\n\nWould you like to install this update now?`
          )

          if (shouldUpdate) {
            try {
              // Download and install with progress logging
              await update.downloadAndInstall(event => {
                switch (event.event) {
                  case 'Started':
                    logger.info(`Downloading ${event.data.contentLength} bytes`)
                    break
                  case 'Progress':
                    logger.info(`Downloaded: ${event.data.chunkLength} bytes`)
                    break
                  case 'Finished':
                    logger.info('Download complete, installing...')
                    break
                }
              })

              // Ask if user wants to restart now
              const shouldRestart = confirm(
                'Update completed successfully!\n\nWould you like to restart the app now to use the new version?'
              )

              if (shouldRestart) {
                await relaunch()
              }
            } catch (updateError) {
              logger.error(`Update installation failed: ${String(updateError)}`)
              alert(
                `Update failed: There was a problem with the automatic download.\n\n${String(updateError)}`
              )
            }
          }
        }
      } catch (checkError) {
        logger.error(`Update check failed: ${String(checkError)}`)
        // Silent fail for update checks - don't bother user with network issues
      }
    }

    // Check for updates 5 seconds after app loads
    const updateTimer = setTimeout(checkForUpdates, 5000)
    return () => clearTimeout(updateTimer)
+  }, [])

-  }, [isWindows])
+  // Set platform attribute once on mount
+  useEffect(() => {
+    // Set platform attribute, windows need a special background color to fix the transparent background
+    // but mac need transparent background to show the window rounded corner
+    // not sure on linux
+    if (isWindows) {
+      document.documentElement.setAttribute('data-platform', 'windows')
+    } else {
+      document.documentElement.setAttribute('data-platform', 'mac')
+    }
+  }, [])
🧹 Nitpick comments (3)
src/quick-pane.css (1)

20-20: Consider whether !important is necessary on #root background.

The !important flag may be required to override other styles, but it's worth verifying whether this is actually needed or if specificity could be adjusted instead.

🔎 Investigate if !important can be removed

Check if there are conflicting background styles that require !important, or if increasing specificity would suffice:

#!/bin/bash
# Description: Search for other background styles applied to #root

# Search for #root background declarations
rg -n --type=css "^#root|background" -A 2 -B 2
src/components/quick-pane/QuickPaneApp.tsx (2)

30-41: Platform detection logic is duplicated from App.tsx.

The applyTheme function sets the data-platform attribute (lines 37-41), which duplicates the exact same logic from App.tsx (lines 25-32). Since QuickPane appears to be a separate window/entry point, this duplication might be intentional. However, consider whether the platform attribute could be set once when the QuickPane window is created, rather than every time applyTheme is called.

🔎 Consider extracting platform setup into a separate effect

Move platform attribute setup out of applyTheme into a separate one-time effect:

 // Apply theme from localStorage to document
 function applyTheme() {
   const theme = localStorage.getItem('ui-theme') || 'system'
   const root = document.documentElement
-  // Check platform
-  const isWindows = getPlatform() === 'windows'
   root.classList.remove('light', 'dark')

-  // Set platform attribute, windows need a special background color to fix the transparent background
-  // but mac need transparent background to show the window rounded corner
-  // not sure on linux
-  if (isWindows) {
-    document.documentElement.setAttribute('data-platform', 'windows')
-  } else {
-    document.documentElement.setAttribute('data-platform', 'mac')
-  }
-
   if (theme === 'system') {
     const systemTheme = window.matchMedia('(prefers-color-scheme: dark)')
       .matches
       ? 'dark'
       : 'light'
     root.classList.add(systemTheme)
   } else {
     root.classList.add(theme)
   }
 }

 export default function QuickPaneApp() {
   const [text, setText] = useState('')
   const inputRef = useRef<HTMLInputElement>(null)
   const isWindows = useIsWindows()
+  
+  // Set platform attribute once on mount
+  useEffect(() => {
+    // Set platform attribute, windows need a special background color to fix the transparent background
+    // but mac need transparent background to show the window rounded corner
+    // not sure on linux
+    if (isWindows) {
+      document.documentElement.setAttribute('data-platform', 'windows')
+    } else {
+      document.documentElement.setAttribute('data-platform', 'mac')
+    }
+  }, [])
+  
   // Apply theme on mount and listen for theme changes from main window
   useEffect(() => {

30-31: Simplify platform detection—use the existing hook.

Line 31 calls getPlatform() === 'windows' directly inside applyTheme, whilst the component already uses useIsWindows() (line 57). If you extract the platform setup into a separate effect (as suggested above), you can eliminate this redundant call and rely solely on the hook.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47e704b and d030c78.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .gitignore
  • src/App.css
  • src/App.tsx
  • src/components/quick-pane/QuickPaneApp.tsx
  • src/quick-pane.css
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}@(!(test|spec)): Use Zustand selector syntax to access state: useUIStore(state => state.value) instead of destructuring to prevent render cascades
Use getState() in callbacks and event handlers to access current state without causing unnecessary re-renders

Files:

  • src/App.tsx
  • src/components/quick-pane/QuickPaneApp.tsx
src/**/*.{tsx,ts}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

Use useTranslation() hook from react-i18next in React components for internationalization

Files:

  • src/App.tsx
  • src/components/quick-pane/QuickPaneApp.tsx
**/*.{tsx,ts,css}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

Use CSS logical properties (text-start, text-end, inset-start) instead of directional properties (text-left, text-right) for RTL language support

Files:

  • src/App.tsx
  • src/quick-pane.css
  • src/App.css
  • src/components/quick-pane/QuickPaneApp.tsx
src/**/*.{ts,tsx}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}@(!(test|spec)): Use type-safe commands from @/lib/tauri-bindings (generated by tauri-specta) instead of string-based invoke() calls
Always handle Tauri command results with result.status === 'ok' check; results use Result pattern, not Promise resolution

Files:

  • src/App.tsx
  • src/components/quick-pane/QuickPaneApp.tsx
{src-tauri/**/*.rs,src/**/*.{ts,tsx}@(!(test|spec))}

📄 CodeRabbit inference engine (AGENTS.md)

Emit events from Rust to React using app.emit("event-name", data) and listen in React with listen("event-name", handler)

Files:

  • src/App.tsx
  • src/components/quick-pane/QuickPaneApp.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T03:24:21.129Z
Learnt from: CR
Repo: dannysmith/tauri-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T03:24:21.129Z
Learning: Applies to src/**/*.{tsx}@(!(test|spec)) : Use React setState for component-local UI state, Zustand for global UI state shared across components, and TanStack Query for persistent data

Applied to files:

  • src/components/quick-pane/QuickPaneApp.tsx
🧬 Code graph analysis (2)
src/App.tsx (1)
src/hooks/use-platform.ts (1)
  • useIsWindows (93-95)
src/components/quick-pane/QuickPaneApp.tsx (1)
src/hooks/use-platform.ts (2)
  • getPlatform (60-62)
  • useIsWindows (93-95)
🪛 Biome (2.1.2)
src/quick-pane.css

[error] 10-10: Unexpected value or character.

Expected one of:

(parse)

src/App.css

[error] 16-16: Unexpected value or character.

Expected one of:

(parse)

🔇 Additional comments (5)
.gitignore (1)

28-28: Pattern addition aligns well with the PR context.

The /img/ pattern correctly ignores the root-level image directory, which is appropriate for excluding image assets (likely before/after screenshots) related to the platform-specific UI fixes introduced in this PR.

src/App.css (2)

41-42: LGTM!

The use of the CSS variable with a transparent fallback is appropriate and aligns with the platform-specific styling requirements.


13-24: CSS variable setup is correct; Biome parse error is a false positive.

The platform-aware CSS variable setup correctly assigns a solid background for Windows and transparent for Mac/default. The theme(colors.background) function is supported in @layer base for global CSS files, so the Biome parse error is a false positive.

src/quick-pane.css (1)

7-18: CSS variable setup mirrors App.css correctly.

The platform-aware CSS variable implementation is consistent with App.css. The Biome parse error on line 10 is the same false positive regarding Tailwind's theme() function.

src/components/quick-pane/QuickPaneApp.tsx (1)

122-128: Conditional styling with clsx is well-implemented.

The use of clsx to conditionally apply rounded-xl only on non-Windows platforms correctly addresses the Windows rounded corner issue described in the PR objectives.

Comment on lines +25 to +32
// Set platform attribute, windows need a special background color to fix the transparent background
// but mac need transparent background to show the window rounded corner
// not sure on linux
if (isWindows) {
document.documentElement.setAttribute('data-platform', 'windows')
} else {
document.documentElement.setAttribute('data-platform', 'mac')
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Platform logic assumes non-Windows platforms are macOS.

The code sets data-platform to 'mac' for all non-Windows platforms (line 31), but the comment on line 27 acknowledges uncertainty about Linux. Consider explicitly handling Linux as a third case or documenting this assumption more clearly.

🔎 Proposed fix to handle Linux explicitly
-    // Set platform attribute, windows need a special background color to fix the transparent background
-    // but mac need transparent background to show the window rounded corner
-    // not sure on linux
-    if (isWindows) {
-      document.documentElement.setAttribute('data-platform', 'windows')
-    } else {
-      document.documentElement.setAttribute('data-platform', 'mac')
-    }
+    // Set platform attribute, windows need a special background color to fix the transparent background
+    // but mac/linux need transparent background to show the window rounded corner
+    const platform = isWindows ? 'windows' : 'unix'
+    document.documentElement.setAttribute('data-platform', platform)

Then update CSS files to use [data-platform='unix'] instead of [data-platform='mac'].

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/App.tsx around lines 25 to 32, the platform logic treats all non-Windows
OSes as macOS; add an explicit branch to detect Linux (or other Unix-like) and
set a distinct data-platform value (e.g., 'linux' or 'unix') instead of always
'mac', leaving 'windows' for Windows and keeping 'mac' for macOS; then update
CSS to target the new value (e.g., [data-platform='unix'] or
[data-platform='linux']) where platform-specific styles currently rely on 'mac'.

@winner106
Copy link

winner106 commented Dec 30, 2025

The problem still exists.
image

image

@DunHouGo
Copy link
Author

this is caused by the darken effect of the Preferences dialog, the DialogOverlay component in shadcn,

you need to modify the dialog.tsx file and change the className of DialogOverlay, I think this is a issue of upstream dependence, you can remove the round-xl in the style and it can be fixed.

not sure if this should modify in the code.

image

fix2: the PreferencesDialog error on Mac, when mouse hover on the origin close button, sidebar will render a stranged state.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/components/preferences/PreferencesDialog.tsx (1)

100-102: Optional: Remove trailing spaces in className strings.

Lines 100 and 101 have trailing spaces in their className strings. While harmless, removing them improves code cleanliness.

🔎 Proposed cleanup
-          <main className="flex flex-1 flex-col overflow-hidden ">
-            <header className="flex h-16 shrink-0 items-center gap-2 ">
+          <main className="flex flex-1 flex-col overflow-hidden">
+            <header className="flex h-16 shrink-0 items-center gap-2">
src/components/ui/dialog.tsx (2)

69-69: Use logical property for RTL support.

The close button positioning uses right-4, which doesn't support right-to-left languages. Per coding guidelines, logical properties should be used for proper RTL layout support.

🔎 Suggested fix
-          className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute top-4 right-4 rounded-xs opacity-70 transition-opacity hover:opacity-100 focus:ring-2 focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4"
+          className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute top-4 end-4 rounded-xs opacity-70 transition-opacity hover:opacity-100 focus:ring-2 focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4"

As per coding guidelines.


72-72: Internationalise the close button label.

The hardcoded "Close" string should be translated using the useTranslation() hook to support multiple languages, as required by the coding guidelines for src/ files.

🔎 Suggested fix

Add the translation hook at the component level and use it for the label:

 >(({ className, children, showCloseButton = true, ...props }, ref) => (
+  const { t } = useTranslation()
+
   <DialogPortal data-slot="dialog-portal">
     <DialogOverlay />
     <DialogPrimitive.Content
           <XIcon />
-          <span className="sr-only">Close</span>
+          <span className="sr-only">{t('common.close')}</span>
         </DialogPrimitive.Close>

As per coding guidelines.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d030c78 and 0a6bf2c.

📒 Files selected for processing (3)
  • src/components/layout/MainWindow.tsx
  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}@(!(test|spec)): Use Zustand selector syntax to access state: useUIStore(state => state.value) instead of destructuring to prevent render cascades
Use getState() in callbacks and event handlers to access current state without causing unnecessary re-renders

Files:

  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
  • src/components/layout/MainWindow.tsx
src/**/*.{tsx,ts}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

Use useTranslation() hook from react-i18next in React components for internationalization

Files:

  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
  • src/components/layout/MainWindow.tsx
**/*.{tsx,ts,css}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

Use CSS logical properties (text-start, text-end, inset-start) instead of directional properties (text-left, text-right) for RTL language support

Files:

  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
  • src/components/layout/MainWindow.tsx
src/**/*.{ts,tsx}@(!(test|spec))

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}@(!(test|spec)): Use type-safe commands from @/lib/tauri-bindings (generated by tauri-specta) instead of string-based invoke() calls
Always handle Tauri command results with result.status === 'ok' check; results use Result pattern, not Promise resolution

Files:

  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
  • src/components/layout/MainWindow.tsx
{src-tauri/**/*.rs,src/**/*.{ts,tsx}@(!(test|spec))}

📄 CodeRabbit inference engine (AGENTS.md)

Emit events from Rust to React using app.emit("event-name", data) and listen in React with listen("event-name", handler)

Files:

  • src/components/preferences/PreferencesDialog.tsx
  • src/components/ui/dialog.tsx
  • src/components/layout/MainWindow.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-18T03:24:21.129Z
Learnt from: CR
Repo: dannysmith/tauri-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T03:24:21.129Z
Learning: Applies to src/**/*.{tsx,ts}@(!(test|spec)) : Use `useTranslation()` hook from react-i18next in React components for internationalization

Applied to files:

  • src/components/preferences/PreferencesDialog.tsx
📚 Learning: 2025-12-18T03:24:21.129Z
Learnt from: CR
Repo: dannysmith/tauri-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T03:24:21.129Z
Learning: Applies to src/**/*.{tsx}@(!(test|spec)) : Use React setState for component-local UI state, Zustand for global UI state shared across components, and TanStack Query for persistent data

Applied to files:

  • src/components/preferences/PreferencesDialog.tsx
📚 Learning: 2025-12-18T03:24:21.129Z
Learnt from: CR
Repo: dannysmith/tauri-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T03:24:21.129Z
Learning: Applies to src/**/*.{ts}@(!(test|spec|*.d.ts)) : For non-React contexts, use `i18n.t.bind(i18n)` for multiple translation calls or call `i18n.t()` directly for occasional use

Applied to files:

  • src/components/preferences/PreferencesDialog.tsx
🧬 Code graph analysis (2)
src/components/preferences/PreferencesDialog.tsx (3)
src/components/ui/dialog.tsx (1)
  • DialogContent (132-132)
src/components/ui/breadcrumb.tsx (1)
  • Breadcrumb (102-102)
src/components/ui/button.tsx (1)
  • Button (60-60)
src/components/layout/MainWindow.tsx (2)
src/hooks/use-platform.ts (1)
  • useIsWindows (93-95)
src/lib/utils.ts (1)
  • cn (4-6)
🔇 Additional comments (4)
src/components/layout/MainWindow.tsx (3)

17-17: LGTM!

The import of useIsWindows is correctly structured and follows the project's hook pattern.


38-38: Platform detection implementation is correct and properly handles all supported platforms.

The useIsWindows() hook correctly detects the Windows platform and is used appropriately to conditionally apply styling. The underlying implementation in use-platform.ts is well-designed with caching for performance, error handling with sensible fallback, and explicit support for Windows, macOS, and Linux.


43-50: The wrapper div is the main window container—not an extra unnecessary layer.

The div at lines 43–50 is the main application window container, not an additional wrapper added for styling. Applying platform-conditional rounding here is the correct approach. DialogOverlay's comment notes "rounded-xl will cause a bad experience on Windows," but this is simply documenting why rounded-xl is not applied to the overlay itself—not indicating a missing fix. DialogOverlay currently applies only the semi-transparent darken effect (bg-black/50); no rounding classes are present there.

The conditional approach (!isWindows && 'rounded-xl') is intentional and appropriate. The QuickPaneApp demonstrates a similar platform-aware pattern using data-platform attributes and CSS variables.

Likely an incorrect or invalid review comment.

src/components/preferences/PreferencesDialog.tsx (1)

3-3: LGTM: Imports properly added for new close button functionality.

The XIcon and Button imports are correctly added and used in the custom close button implementation.

Also applies to: 32-32

return (
<Dialog open={preferencesOpen} onOpenChange={setPreferencesOpen}>
<DialogContent className="overflow-hidden p-0 md:max-h-[600px] md:max-w-[900px] lg:max-w-[1000px] font-sans rounded-xl">
<DialogContent showCloseButton={false} className="overflow-hidden p-0 md:max-h-[600px] md:max-w-[900px] lg:max-w-[1000px] font-sans rounded-xl">
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the DialogOverlay implementation and its styling

# First, find the dialog.tsx file
find . -name "dialog.tsx" -type f | head -20

# Check the DialogOverlay component definition and styling
rg -n "DialogOverlay|rounded|round" src/components/ui/dialog.tsx -C3

Repository: dannysmith/tauri-template

Length of output: 2537


Remove rounded-xl or use rounded-lg to align with the Windows artifact fix.

The dialog.tsx file explicitly notes that rounded-xl causes rendering issues on Windows and has removed rounding from the DialogOverlay. However, this change adds rounded-xl to the DialogContent at line 66, which overrides the default rounded-lg and reintroduces the problematic rounding level. Use rounded-lg instead, or remove the rounding class entirely to match the default DialogContent styling.

🤖 Prompt for AI Agents
In src/components/preferences/PreferencesDialog.tsx around line 66, the
DialogContent uses className including "rounded-xl" which reintroduces a Windows
rendering issue; change "rounded-xl" to "rounded-lg" or remove the rounding
class entirely so it matches the DialogOverlay/default DialogContent styling and
avoids the problematic corner radius. Ensure the final className no longer
contains "rounded-xl" and update any tests or snapshots if they rely on the
previous class.

Comment on lines +118 to +120
<Button variant="ghost" onClick={() => setPreferencesOpen(false)}>
<XIcon className="h-4 w-4" />
</Button>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add accessible label to the close button for screen reader users.

The close button lacks an accessible name for assistive technologies. Screen reader users will only hear "button" without understanding its purpose.

🔎 Recommended fix
-                <Button variant="ghost" onClick={() => setPreferencesOpen(false)}>
+                <Button
+                  variant="ghost"
+                  onClick={() => setPreferencesOpen(false)}
+                  aria-label={t('preferences.close')}
+                >
                  <XIcon className="h-4 w-4" />
                </Button>

Note: You'll need to add the corresponding translation key preferences.close to your i18n locale files.

🤖 Prompt for AI Agents
In src/components/preferences/PreferencesDialog.tsx around lines 118 to 120, the
close Button has no accessible name so screen readers only announce "button";
add an explicit accessible label by passing an aria-label (or aria-labelledby)
prop using the i18n translation key (e.g., t('preferences.close')) to the Button
component, and add the corresponding preferences.close entry to the i18n locale
files.

Comment on lines +39 to +40
// rounded-xl will cause a bad experience on Windows
'data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 fixed inset-0 z-50 bg-black/50',
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read the dialog.tsx file to verify the current state and check for guideline violations
wc -l src/components/ui/dialog.tsx

Repository: dannysmith/tauri-template

Length of output: 101


🏁 Script executed:

# Read the entire dialog.tsx file with line numbers to verify lines 39-40 and check for violations at 69, 72
cat -n src/components/ui/dialog.tsx

Repository: dannysmith/tauri-template

Length of output: 5608


🏁 Script executed:

# Search for DialogOverlay usage elsewhere in the codebase
rg -n --type=tsx --type=ts 'DialogOverlay' --glob='!src/components/ui/dialog.tsx'

Repository: dannysmith/tauri-template

Length of output: 96


🏁 Script executed:

# Search for DialogOverlay usage elsewhere (retry with correct syntax)
rg -n 'DialogOverlay' --type ts --type tsx -- . | grep -v 'src/components/ui/dialog.tsx'

Repository: dannysmith/tauri-template

Length of output: 96


🏁 Script executed:

# Also check for any i18n setup in the project
fd -e ts -e tsx | head -20 | xargs grep -l "useTranslation\|i18n" 2>/dev/null | head -5

Repository: dannysmith/tauri-template

Length of output: 275


🏁 Script executed:

# Check what's at the root level to understand project structure
git ls-files | grep -E '(i18n|translation|locale)' | head -10

Repository: dannysmith/tauri-template

Length of output: 230


🏁 Script executed:

# Check for DialogOverlay usage without type flags
rg 'DialogOverlay' -- src/

Repository: dannysmith/tauri-template

Length of output: 513


🏁 Script executed:

# Check if other components use i18n for accessibility labels
grep -r "sr-only" src/ --include="*.tsx" -A2 -B2 | head -40

Repository: dannysmith/tauri-template

Length of output: 2979


🏁 Script executed:

# Check the i18n config to understand structure
cat src/i18n/config.ts

Repository: dannysmith/tauri-template

Length of output: 1187


🏁 Script executed:

# Check if there's a locale file to see if "Close" is translated
grep -i "close" locales/en.json

Repository: dannysmith/tauri-template

Length of output: 256


🏁 Script executed:

# Check if there's already a translation key for a generic "Close" button
grep -i '"close"' locales/en.json

Repository: dannysmith/tauri-template

Length of output: 51


🏁 Script executed:

# Verify the exact context of line 69 to confirm the directional property issue
sed -n '67,71p' src/components/ui/dialog.tsx

Repository: dannysmith/tauri-template

Length of output: 540


🏁 Script executed:

# Check if there are other uses of right-4, left-4 in dialog components that should also be fixed
rg 'right-[0-9]|left-[0-9]' src/components/ui/dialog.tsx

Repository: dannysmith/tauri-template

Length of output: 444


🏁 Script executed:

# Check PreferencesDialog to see how it properly handles i18n for close buttons
grep -A5 -B5 'Close\|close' src/components/preferences/PreferencesDialog.tsx | head -30

Repository: dannysmith/tauri-template

Length of output: 540


LGTM for lines 39-40, but address guideline violations in the same component.

The removal of rounded-xl from DialogOverlay is correct. Since the overlay spans the entire viewport (inset-0), rounded corners wouldn't be visible anyway, and this fixes the Windows rendering artifacts described in the PR.

However, this component has two guideline violations:

  • Line 69: Replace directional property right-4 with logical property inset-end-4 for RTL language support.
  • Line 72: Replace hardcoded "Close" text with useTranslation() hook. The project has i18n configured with locale files; use the pattern shown in PreferencesDialog.tsx.
🤖 Prompt for AI Agents
In src/components/ui/dialog.tsx around lines 69 and 72, replace the directional
Tailwind class and the hardcoded label to meet guidelines: change the class
'right-4' to the logical 'inset-end-4' to support RTL layouts, and replace the
literal "Close" string with the i18n pattern used in PreferencesDialog.tsx by
importing useTranslation, calling const { t } = useTranslation(), and using
t('close') (or the appropriate key from your locale files) for the button label;
ensure you add the import at the top and use the same translation
namespace/pattern as PreferencesDialog.

Copy link

@winner106 winner106 left a comment

Choose a reason for hiding this comment

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

Great work, perfect!

@dannysmith
Copy link
Owner

Heya! Only just seen this - thanks @DunHouGo. Gimme a few days and I'll look over this before merging.

I recently dealt with something similar in Astro Editor (the project which inspired this template).

Also: Apologies for the overly-noisy CodeRabbit. Just opened #14 to deal with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants