Skip to content

Conversation

@Hemil36
Copy link
Contributor

@Hemil36 Hemil36 commented Sep 13, 2025

  • Fixes : Refactor Settings.tsx #509
    This pull request introduces a set of new custom React hooks and refactors several settings page components to improve modularity, feedback handling, and user experience for folder management, user preferences, and application controls. The changes focus on abstracting logic for folder operations and user preferences into reusable hooks, standardizing mutation feedback, and creating reusable UI components for consistent styling.

Hooks and Feedback Handling

  • Added useFolderOperations hook to encapsulate folder queries, AI tagging toggles, and folder deletion, including standardized mutation feedback and Redux integration for folder state.
  • Added useUserPreferences hook to manage user preferences state, querying, updating, and feedback for settings like YOLO model size and GPU acceleration.
  • Introduced useMutationFeedback hook to provide consistent UI feedback (loading, success, error) for mutations using Redux actions and dialogs.

Settings Page Component Refactor

  • Refactored FolderManagementCard, UserPreferencesCard, and ApplicationControlsCard components to use the new hooks, improving separation of concerns and simplifying UI logic.
  • Added a reusable SettingsCard component for consistent card styling and layout across all settings sections.

Summary by CodeRabbit

  • New Features

    • Redesigned Settings page into modular cards: Folder Management, User Preferences, and Application Controls.
    • Folder Management: view configured folders, toggle AI tagging, delete folders, and add new folders.
    • User Preferences: adjust YOLO model size (Nano/Small/Medium) and toggle GPU acceleration.
    • Application Controls: check for updates with guided dialog and restart the server.
    • Improved UI feedback for loading, success, and errors during actions.
  • Refactor

    • Moved data fetching and state management into reusable hooks and components for a cleaner, more responsive Settings experience.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 13, 2025

Walkthrough

Introduces three reusable hooks (folder operations, mutation feedback, user preferences), refactors Settings into modular cards, and adds three Settings subcomponents plus a shared SettingsCard. Application controls integrate update checking and server restart. Folder management and user preferences UIs now consume the new hooks. Public exports added accordingly.

Changes

Cohort / File(s) Summary
Hooks: folder ops, mutation feedback, user prefs
frontend/src/hooks/useFolderOperations.tsx, frontend/src/hooks/useMutationFeedback.tsx, frontend/src/hooks/useUserPreferences.tsx
Add hooks for folder CRUD/toggles with React Query + Redux sync, standardized mutation UI feedback, and user preferences fetch/update with helper methods and pending/loading states.
Settings orchestrator
frontend/src/pages/SettingsPage/Settings.tsx
Replaces inline logic/UI with three modular cards. Removes direct queries/mutations, update flows, and assorted imports. Keeps same default export signature.
Settings components
frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx, .../UserPreferencesCard.tsx, .../ApplicationControlsCard.tsx, .../SettingsCard.tsx
Add modular cards: FolderManagement uses useFolderOperations; UserPreferences uses useUserPreferences; ApplicationControls uses useUpdater, UpdateDialog, and Redux loader/dialog; SettingsCard provides shared card layout.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant FMC as FolderManagementCard
  participant H as useFolderOperations
  participant API as Folders API
  participant RQ as React Query
  participant RS as Redux Store

  U->>FMC: Load Settings
  FMC->>H: useFolderOperations()
  H->>RQ: Query: getFolders
  RQ-->>H: folders[]
  H->>RS: Sync folders slice
  FMC-->>U: Render folders

  U->>FMC: Toggle AI Tagging
  FMC->>H: toggleAITagging(folder)
  H->>API: enable/disable AI tagging
  API-->>H: success
  H->>RQ: Invalidate getFolders
  RQ-->>H: Refetch folders
  H->>RS: Update folders slice
  FMC-->>U: Updated state

  U->>FMC: Delete folder
  FMC->>H: deleteFolder(id)
  H->>API: delete folder
  API-->>H: success
  H->>RQ: Invalidate getFolders
  RQ-->>H: Refetch & sync
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant ACC as ApplicationControlsCard
  participant UP as useUpdater
  participant D as UpdateDialog
  participant RL as Redux Loader/Dialog

  U->>ACC: Click "Check for Updates"
  ACC->>RL: showLoader
  ACC->>UP: checkForUpdates()
  alt Update available
    UP-->>ACC: updateAvailable=true
    ACC->>D: open UpdateDialog
  else No update
    UP-->>ACC: updateAvailable=false
    ACC->>RL: showInfoDialog("No Updates Available")
  end
  ACC->>RL: hideLoader

  U->>D: Download
  D->>UP: downloadAndInstall()
  UP-->>D: progress / complete

  U->>ACC: Click "Restart Server"
  ACC->>UP: restartServer()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • Refactor Settings.tsx #509 — Implements the requested refactor by extracting hooks and modularizing the Settings page into subcomponents.

Possibly related PRs

Suggested labels

GSoC 2025

Poem

A nibble of code, a hop to refactor neat,
Cards in a garden where settings now meet.
Toggles that twinkle, updates that sing,
Preferences burrow on a gentle spring.
With hooks in our paws, we tidy the lair—
Thump-thump! Ship it, with cotton-tailed care. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the primary change: a refactor of the Settings page to add folder and user-preferences management. It is concise, focused on the main change described in the PR objectives, and avoids noisy details or irrelevant items.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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
Contributor

@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: 7

🧹 Nitpick comments (7)
frontend/src/hooks/useUserPreferences.tsx (1)

45-52: Reuse hook callbacks to reduce duplication

Since the hook already exposes domain helpers, consider using an onError in useMutationFeedback to perform the same rollback (or trigger preferencesQuery.refetch) to keep concerns centralized.

frontend/src/hooks/useFolderOperations.tsx (1)

133-137: Expose per-folder pending to avoid global toggle disable

Disabling all toggles when any mutation is pending degrades UX. Track the active folderId per mutation (e.g., via useRef/state in the hook) and expose enablePendingId/disablePendingId so only the relevant row disables.

frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx (1)

33-37: Fix label association for non-form control

Label htmlFor points to “yolo-model” but the dropdown trigger isn’t a form control with that id. Remove htmlFor to avoid invalid association (or switch to a Select component with an input).

Apply this diff:

-            <Label
-              htmlFor="yolo-model"
-              className="text-foreground text-sm font-medium"
-            >
+            <Label className="text-foreground text-sm font-medium">
               YOLO Model Size
             </Label>
frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (1)

62-70: Confirm destructive action before deleting

Add a simple confirmation to prevent accidental deletions.

Apply this diff:

-                  <Button
-                    onClick={() => deleteFolder(folder.folder_id)}
+                  <Button
+                    onClick={() => {
+                      if (window.confirm('Remove this folder from your library?')) {
+                        deleteFolder(folder.folder_id);
+                      }
+                    }}
                     variant="outline"
                     size="sm"
frontend/src/pages/SettingsPage/components/ApplicationControlsCard.tsx (1)

106-107: Simplify boolean expression

false || !isDownloading → !isDownloading.

Apply this diff:

-        showCloseButton={false || !isDownloading}
+        showCloseButton={!isDownloading}
frontend/src/pages/SettingsPage/Settings.tsx (2)

1-1: Optional: code-split the cards with React.lazy + Suspense for a small route load-time win

Swapping static imports for lazy ones and wrapping the trio in a single Suspense keeps the page responsive while each card loads. Also drops the now-unnecessary default React import.

-import React from 'react';
+import { Suspense, lazy } from 'react';

-// Import modular components
-import FolderManagementCard from './components/FolderManagementCard';
-import UserPreferencesCard from './components/UserPreferencesCard';
-import ApplicationControlsCard from './components/ApplicationControlsCard';
+// Lazy-load modular components
+const FolderManagementCard = lazy(() => import('./components/FolderManagementCard'));
+const UserPreferencesCard = lazy(() => import('./components/UserPreferencesCard'));
+const ApplicationControlsCard = lazy(() => import('./components/ApplicationControlsCard'));
@@
-        {/* Folder Management */}
-        <FolderManagementCard />
-
-        {/* User Preferences */}
-        <UserPreferencesCard />
-
-        {/* Application Controls */}
-        <ApplicationControlsCard />
+        <Suspense fallback={<div className="py-2 text-sm text-gray-500">Loading settings…</div>}>
+          {/* Folder Management */}
+          <FolderManagementCard />
+
+          {/* User Preferences */}
+          <UserPreferencesCard />
+
+          {/* Application Controls */}
+          <ApplicationControlsCard />
+        </Suspense>

Also applies to: 3-6, 16-23


16-23: Consider minor a11y semantics: use a

landmark and section headings

Wrapping the outer container in a semantic

(or adding role="main") and ensuring each card exposes an accessible heading improves navigation for screen readers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d71a3e7 and 3aa46ba.

📒 Files selected for processing (8)
  • frontend/src/hooks/useFolderOperations.tsx (1 hunks)
  • frontend/src/hooks/useMutationFeedback.tsx (1 hunks)
  • frontend/src/hooks/useUserPreferences.tsx (1 hunks)
  • frontend/src/pages/SettingsPage/Settings.tsx (1 hunks)
  • frontend/src/pages/SettingsPage/components/ApplicationControlsCard.tsx (1 hunks)
  • frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (1 hunks)
  • frontend/src/pages/SettingsPage/components/SettingsCard.tsx (1 hunks)
  • frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/src/hooks/useUserPreferences.tsx (3)
frontend/src/hooks/useQueryExtension.ts (2)
  • usePictoQuery (80-108)
  • usePictoMutation (26-78)
frontend/src/api/api-functions/user_preferences.ts (2)
  • getUserPreferences (25-31)
  • updateUserPreferences (33-41)
frontend/src/hooks/useMutationFeedback.tsx (1)
  • useMutationFeedback (60-135)
frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx (1)
frontend/src/hooks/useUserPreferences.tsx (1)
  • useUserPreferences (14-97)
frontend/src/pages/SettingsPage/components/ApplicationControlsCard.tsx (4)
frontend/src/hooks/useUpdater.ts (1)
  • useUpdater (24-138)
frontend/src/features/loaderSlice.ts (2)
  • showLoader (17-20)
  • hideLoader (21-24)
frontend/src/features/infoDialogSlice.ts (1)
  • showInfoDialog (16-30)
frontend/src/utils/serverUtils.ts (1)
  • restartServer (56-66)
frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (2)
frontend/src/hooks/useFolderOperations.tsx (1)
  • useFolderOperations (19-138)
frontend/src/components/FolderPicker/FolderPicker.tsx (1)
  • FolderPicker (5-29)
frontend/src/hooks/useMutationFeedback.tsx (2)
frontend/src/features/loaderSlice.ts (2)
  • showLoader (17-20)
  • hideLoader (21-24)
frontend/src/features/infoDialogSlice.ts (1)
  • showInfoDialog (16-30)
frontend/src/hooks/useFolderOperations.tsx (5)
frontend/src/features/folderSelectors.ts (1)
  • selectAllFolders (8-8)
frontend/src/hooks/useQueryExtension.ts (2)
  • usePictoQuery (80-108)
  • usePictoMutation (26-78)
frontend/src/api/api-functions/folders.ts (4)
  • getAllFolders (26-31)
  • enableAITagging (43-51)
  • disableAITagging (53-61)
  • deleteFolders (63-71)
frontend/src/hooks/useMutationFeedback.tsx (1)
  • useMutationFeedback (60-135)
frontend/src/features/folderSlice.ts (1)
  • setFolders (17-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Backend Tests
  • GitHub Check: Tauri Build Check (windows-latest)
  • GitHub Check: Tauri Tests
  • GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
  • GitHub Check: Tauri Build Check (ubuntu-22.04)
🔇 Additional comments (3)
frontend/src/pages/SettingsPage/components/SettingsCard.tsx (1)

26-46: LGTM – clean, reusable layout

Component is simple and consistent. No issues spotted.

frontend/src/pages/SettingsPage/Settings.tsx (2)

8-11: JSDoc is clear and accurate

Good, concise description of the component’s role as an orchestrator.


17-17: Verified — default exports present for SettingsPage cards

Confirmed: FolderManagementCard, UserPreferencesCard, and ApplicationControlsCard all export default; no changes required.

@Hemil36 Hemil36 marked this pull request as draft September 14, 2025 11:30
@Hemil36 Hemil36 marked this pull request as ready for review September 16, 2025 17:42
@Hemil36 Hemil36 marked this pull request as draft September 17, 2025 06:07
@Hemil36 Hemil36 marked this pull request as ready for review September 20, 2025 18:06
@rahulharpal1603
Copy link
Contributor

@Hemil36

Thanks for the PR!

@rahulharpal1603 rahulharpal1603 merged commit 6a1458c into AOSSIE-Org:main Sep 21, 2025
7 checks passed
@Hemil36
Copy link
Contributor Author

Hemil36 commented Sep 21, 2025

Hi @rahulharpal1603 , thanks for reviewing and merging my PR!
I noticed that the merge commit shows your name as the author. To help preserve my authorship and keep the history clean, would it be possible to use the “Rebase and merge” option when merging my future PRs?
That way, my commits stay directly in the main branch under my name, and the contribution trace is clearer.
Totally fine if the project prefers another workflow — I just wanted to check!

@rahulharpal1603
Copy link
Contributor

rahulharpal1603 commented Sep 21, 2025

https://github.com/AOSSIE-Org/PictoPy/commits/main/

Your commits are always there(See the image below, the 2 commits you made in this PR are listed in the picture). The commit with my author name is an additional commit that is recorded automatically while merging to record who merged the PR.
Image

@rahulharpal1603
Copy link
Contributor

We can also rebase and merge. I will look into that. In any case the authorship is always preserved.

@Hemil36
Copy link
Contributor Author

Hemil36 commented Sep 21, 2025

Yes, I’m aware that authorship is always preserved — I just mentioned rebase and merge since it keeps the history a bit cleaner. Totally fine with whichever workflow the project prefers, and I appreciate you considering it!

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

Labels

enhancement New feature or request frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor Settings.tsx

2 participants