-
Notifications
You must be signed in to change notification settings - Fork 928
Router refactor 2: Settings page cleanup #732
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,14 +1,69 @@ | ||||||||||||||||||||||||
| import { Avatar } from "@superset/ui/atoms/Avatar"; | ||||||||||||||||||||||||
| import { Button } from "@superset/ui/button"; | ||||||||||||||||||||||||
| import { Skeleton } from "@superset/ui/skeleton"; | ||||||||||||||||||||||||
| import { toast } from "@superset/ui/sonner"; | ||||||||||||||||||||||||
| import { createFileRoute } from "@tanstack/react-router"; | ||||||||||||||||||||||||
| import { trpc } from "renderer/lib/trpc"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export const Route = createFileRoute("/_authenticated/settings/account/")({ | ||||||||||||||||||||||||
| component: AccountSettingsPage, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function AccountSettingsPage() { | ||||||||||||||||||||||||
| const { data: user, isLoading } = trpc.user.me.useQuery(); | ||||||||||||||||||||||||
| const signOutMutation = trpc.auth.signOut.useMutation({ | ||||||||||||||||||||||||
| onSuccess: () => toast.success("Signed out"), | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const signOut = () => signOutMutation.mutate(); | ||||||||||||||||||||||||
|
Comment on lines
+14
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for sign-out mutation. The sign-out mutation only handles 🐛 Proposed fix const signOutMutation = trpc.auth.signOut.useMutation({
onSuccess: () => toast.success("Signed out"),
+ onError: (error) => toast.error(`Sign out failed: ${error.message}`),
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||
| <h2>Account Settings</h2> | ||||||||||||||||||||||||
| <p>Account settings placeholder</p> | ||||||||||||||||||||||||
| <div className="p-6 max-w-4xl"> | ||||||||||||||||||||||||
| <div className="mb-8"> | ||||||||||||||||||||||||
| <h2 className="text-xl font-semibold">Account</h2> | ||||||||||||||||||||||||
| <p className="text-sm text-muted-foreground mt-1"> | ||||||||||||||||||||||||
| Manage your account settings | ||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| <div className="space-y-8"> | ||||||||||||||||||||||||
| {/* Profile Section */} | ||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||
| <h3 className="text-sm font-medium mb-4">Profile</h3> | ||||||||||||||||||||||||
| <div className="flex items-center gap-4 p-4 rounded-lg border bg-card"> | ||||||||||||||||||||||||
| {isLoading ? ( | ||||||||||||||||||||||||
| <> | ||||||||||||||||||||||||
| <Skeleton className="h-16 w-16 rounded-full" /> | ||||||||||||||||||||||||
| <div className="space-y-2"> | ||||||||||||||||||||||||
| <Skeleton className="h-5 w-32" /> | ||||||||||||||||||||||||
| <Skeleton className="h-4 w-48" /> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| </> | ||||||||||||||||||||||||
| ) : user ? ( | ||||||||||||||||||||||||
| <> | ||||||||||||||||||||||||
| <Avatar size="xl" fullName={user.name} image={user.image} /> | ||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||
| <p className="font-medium text-lg">{user.name}</p> | ||||||||||||||||||||||||
| <p className="text-sm text-muted-foreground">{user.email}</p> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| </> | ||||||||||||||||||||||||
| ) : ( | ||||||||||||||||||||||||
| <p className="text-muted-foreground">Unable to load user info</p> | ||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| {/* Sign Out Section */} | ||||||||||||||||||||||||
| <div className="pt-6 border-t"> | ||||||||||||||||||||||||
| <h3 className="text-sm font-medium mb-2">Sign Out</h3> | ||||||||||||||||||||||||
| <p className="text-sm text-muted-foreground mb-4"> | ||||||||||||||||||||||||
| Sign out of your Superset account on this device. | ||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||
| <Button variant="outline" onClick={() => signOut()}> | ||||||||||||||||||||||||
| Sign Out | ||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { ThemeCard } from "./ThemeCard"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,91 @@ | ||
| import { | ||
| Select, | ||
| SelectContent, | ||
| SelectItem, | ||
| SelectTrigger, | ||
| SelectValue, | ||
| } from "@superset/ui/select"; | ||
| import { createFileRoute } from "@tanstack/react-router"; | ||
| import { | ||
| type MarkdownStyle, | ||
| useMarkdownStyle, | ||
| useSetMarkdownStyle, | ||
| useSetTheme, | ||
| useThemeId, | ||
| useThemeStore, | ||
| } from "renderer/stores"; | ||
| import { builtInThemes } from "shared/themes"; | ||
| import { ThemeCard } from "./components/ThemeCard"; | ||
|
|
||
| export const Route = createFileRoute("/_authenticated/settings/appearance/")({ | ||
| component: AppearanceSettingsPage, | ||
| }); | ||
|
|
||
| function AppearanceSettingsPage() { | ||
| const activeThemeId = useThemeId(); | ||
| const setTheme = useSetTheme(); | ||
| const customThemes = useThemeStore((state) => state.customThemes); | ||
| const markdownStyle = useMarkdownStyle(); | ||
| const setMarkdownStyle = useSetMarkdownStyle(); | ||
|
|
||
| const allThemes = [...builtInThemes, ...customThemes]; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h2>Appearance Settings</h2> | ||
| <p>Appearance settings placeholder</p> | ||
| <div className="p-6 max-w-4xl"> | ||
| <div className="mb-8"> | ||
| <h2 className="text-xl font-semibold">Appearance</h2> | ||
| <p className="text-sm text-muted-foreground mt-1"> | ||
| Customize how Superset looks on your device | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="space-y-8"> | ||
| {/* Theme Section */} | ||
| <div> | ||
| <h3 className="text-sm font-medium mb-4">Theme</h3> | ||
| <div className="grid grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| {allThemes.map((theme) => ( | ||
| <ThemeCard | ||
| key={theme.id} | ||
| theme={theme} | ||
| isSelected={activeThemeId === theme.id} | ||
| onSelect={() => setTheme(theme.id)} | ||
| /> | ||
| ))} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="pt-6 border-t"> | ||
| <h3 className="text-sm font-medium mb-2">Markdown Style</h3> | ||
| <p className="text-sm text-muted-foreground mb-4"> | ||
| Rendering style for markdown files when viewing rendered content | ||
| </p> | ||
| <Select | ||
| value={markdownStyle} | ||
| onValueChange={(value) => setMarkdownStyle(value as MarkdownStyle)} | ||
| > | ||
| <SelectTrigger className="w-[200px]"> | ||
| <SelectValue /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="default">Default</SelectItem> | ||
| <SelectItem value="tufte">Tufte</SelectItem> | ||
| </SelectContent> | ||
| </Select> | ||
| <p className="text-xs text-muted-foreground mt-2"> | ||
| Tufte style uses elegant serif typography inspired by Edward Tufte's | ||
| books | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="pt-6 border-t"> | ||
| <h3 className="text-sm font-medium mb-2">Custom Themes</h3> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Custom theme import coming soon. You'll be able to import JSON theme | ||
| files to create your own themes. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,135 @@ | ||
| import type { TerminalLinkBehavior } from "@superset/local-db"; | ||
| import { Label } from "@superset/ui/label"; | ||
| import { | ||
| Select, | ||
| SelectContent, | ||
| SelectItem, | ||
| SelectTrigger, | ||
| SelectValue, | ||
| } from "@superset/ui/select"; | ||
| import { Switch } from "@superset/ui/switch"; | ||
| import { createFileRoute } from "@tanstack/react-router"; | ||
| import { trpc } from "renderer/lib/trpc"; | ||
|
|
||
| export const Route = createFileRoute("/_authenticated/settings/behavior/")({ | ||
| component: BehaviorSettingsPage, | ||
| }); | ||
|
|
||
| function BehaviorSettingsPage() { | ||
| const utils = trpc.useUtils(); | ||
|
|
||
| // Confirm on quit setting | ||
| const { data: confirmOnQuit, isLoading: isConfirmLoading } = | ||
| trpc.settings.getConfirmOnQuit.useQuery(); | ||
| const setConfirmOnQuit = trpc.settings.setConfirmOnQuit.useMutation({ | ||
| onMutate: async ({ enabled }) => { | ||
| await utils.settings.getConfirmOnQuit.cancel(); | ||
| const previous = utils.settings.getConfirmOnQuit.getData(); | ||
| utils.settings.getConfirmOnQuit.setData(undefined, enabled); | ||
| return { previous }; | ||
| }, | ||
| onError: (_err, _vars, context) => { | ||
| if (context?.previous !== undefined) { | ||
| utils.settings.getConfirmOnQuit.setData(undefined, context.previous); | ||
| } | ||
| }, | ||
| onSettled: () => { | ||
| utils.settings.getConfirmOnQuit.invalidate(); | ||
| }, | ||
| }); | ||
|
|
||
| const handleConfirmToggle = (enabled: boolean) => { | ||
| setConfirmOnQuit.mutate({ enabled }); | ||
| }; | ||
|
|
||
| // Terminal link behavior setting | ||
| const { data: terminalLinkBehavior, isLoading: isLoadingLinkBehavior } = | ||
| trpc.settings.getTerminalLinkBehavior.useQuery(); | ||
|
|
||
| const setTerminalLinkBehavior = | ||
| trpc.settings.setTerminalLinkBehavior.useMutation({ | ||
| onMutate: async ({ behavior }) => { | ||
| await utils.settings.getTerminalLinkBehavior.cancel(); | ||
| const previous = utils.settings.getTerminalLinkBehavior.getData(); | ||
| utils.settings.getTerminalLinkBehavior.setData(undefined, behavior); | ||
| return { previous }; | ||
| }, | ||
| onError: (_err, _vars, context) => { | ||
| if (context?.previous !== undefined) { | ||
| utils.settings.getTerminalLinkBehavior.setData( | ||
| undefined, | ||
| context.previous, | ||
| ); | ||
| } | ||
| }, | ||
| onSettled: () => { | ||
| utils.settings.getTerminalLinkBehavior.invalidate(); | ||
| }, | ||
| }); | ||
|
|
||
| const handleLinkBehaviorChange = (value: string) => { | ||
| setTerminalLinkBehavior.mutate({ | ||
| behavior: value as TerminalLinkBehavior, | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <h2>Behavior Settings</h2> | ||
| <p>Behavior settings placeholder</p> | ||
| <div className="p-6 max-w-4xl w-full"> | ||
| <div className="mb-8"> | ||
| <h2 className="text-xl font-semibold">Behavior</h2> | ||
| <p className="text-sm text-muted-foreground mt-1"> | ||
| Configure app behavior and preferences | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="space-y-6"> | ||
| {/* Confirm on Quit */} | ||
| <div className="flex items-center justify-between"> | ||
| <div className="space-y-0.5"> | ||
| <Label htmlFor="confirm-on-quit" className="text-sm font-medium"> | ||
| Confirm before quitting | ||
| </Label> | ||
| <p className="text-xs text-muted-foreground"> | ||
| Show a confirmation dialog when quitting the app | ||
| </p> | ||
| </div> | ||
| <Switch | ||
| id="confirm-on-quit" | ||
| checked={confirmOnQuit ?? true} | ||
| onCheckedChange={handleConfirmToggle} | ||
| disabled={isConfirmLoading || setConfirmOnQuit.isPending} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="flex items-center justify-between"> | ||
| <div className="space-y-0.5"> | ||
| <Label | ||
| htmlFor="terminal-link-behavior" | ||
| className="text-sm font-medium" | ||
| > | ||
| Terminal file links | ||
| </Label> | ||
| <p className="text-xs text-muted-foreground"> | ||
| Choose how to open file paths when Cmd+clicking in the terminal | ||
| </p> | ||
| </div> | ||
| <Select | ||
| value={terminalLinkBehavior ?? "external-editor"} | ||
| onValueChange={handleLinkBehaviorChange} | ||
| disabled={ | ||
| isLoadingLinkBehavior || setTerminalLinkBehavior.isPending | ||
| } | ||
| > | ||
| <SelectTrigger className="w-[180px]"> | ||
| <SelectValue /> | ||
| </SelectTrigger> | ||
| <SelectContent> | ||
| <SelectItem value="external-editor">External editor</SelectItem> | ||
| <SelectItem value="file-viewer">File viewer</SelectItem> | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated baseBranch detection logic.
This entire block (Lines 35-74) is duplicated nearly verbatim in
getActive(Lines 232-271). Extract to a shared helper to improve maintainability:♻️ Proposed refactor
🤖 Prompt for AI Agents