feat(mobile): random mobile progress#1349
Conversation
📝 WalkthroughWalkthroughUpdates React/react-dom to 19.2.0 and Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/mobile/app.config.ts (1)
52-52:⚠️ Potential issue | 🟡 MinorTypo in
owner:"supserset-sh"→"superset-sh".This appears to be a pre-existing typo ("supserset" instead of "superset"), but it could affect EAS build ownership resolution.
Proposed fix
- owner: "supserset-sh", + owner: "superset-sh",
🤖 Fix all issues with AI agents
In `@apps/mobile/modules/tab-bar/ios/ExpandedMenuView.swift`:
- Around line 73-74: The Divider's .background modifier won't tint the divider
line; replace the current Divider().background(Color.white.opacity(0.2)) with a
divider-styling that actually colors the line—either use an overlay (e.g.,
Divider().overlay(Rectangle().foregroundColor(Color.white.opacity(0.2))).frame(height:
1)) or swap the Divider for a Rectangle directly
(Rectangle().foregroundColor(Color.white.opacity(0.2)).frame(height: 1)); update
the code in ExpandedMenuView where Divider is used to one of these options so
the visible line color matches the intended semi-transparent white.
In `@apps/mobile/modules/tab-bar/ios/TabBar.podspec`:
- Line 15: The Podspec's s.source is pointing to the personal fork
(nicksupersetsh/superset.git); update the s.source value in TabBar.podspec to
point to the canonical repository (superset-sh/superset.git) so the pod resolves
from the organization repo (change the git URL assigned to s.source
accordingly).
In `@apps/mobile/screens/`(auth)/sign-in/SignInScreen.tsx:
- Around line 9-10: The onPress handlers that call Linking.openURL with
TERMS_URL and PRIVACY_URL can reject and currently fire-and-forget; add a small
helper (e.g., openURL) that calls Linking.openURL(url).catch(err =>
console.error("[sign-in] Failed to open URL:", url, err)) and replace direct
Linking.openURL calls in the SignInScreen onPress callbacks with this helper to
ensure promise rejections are logged instead of swallowed.
In `@apps/mobile/screens/`(authenticated)/(home)/workspaces/WorkspacesScreen.tsx:
- Around line 29-32: onRefresh is currently a no-op because it sets
setRefreshing(true) then immediately setRefreshing(false); update useCallback
onRefresh to perform the actual refresh by invoking the workspace data
loader/refetch function (e.g., call loadWorkspaces, fetchWorkspaces, or
refetchWorkspaces) and await its result, and ensure setRefreshing(false) runs in
a finally block so the spinner stops after the fetch completes; if this is
intentional scaffolding, replace the body with a clear TODO comment referencing
onRefresh and setRefreshing to indicate the refresh implementation is pending.
In `@apps/mobile/screens/`(authenticated)/(more)/MoreMenuScreen.tsx:
- Around line 25-50: MoreMenuScreen duplicates organization logic (session, orgs
query, activeOrg resolution, handleSwitchOrg and switching state) that the
shared useOrganizations hook already encapsulates; refactor MoreMenuScreen to
import and use useOrganizations instead of re-implementing that logic, move the
local switching state into the shared hook (or expose a switching setter and
switchOrg method) so MoreMenuScreen can call the hook's switch function (replace
handleSwitchOrg), consume activeOrg, otherOrgs, and switching from the hook, and
remove the duplicate useLiveQuery/session code—ensure the hook API matches what
OrgDropdown expects so both components reuse the same functions and state.
In
`@apps/mobile/screens/`(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.tsx:
- Around line 34-49: The collapse timeout (collapseTimer used inside
handleExpandedChange) is not cleared on unmount, risking a leaked timer and a
stale setIsExpanded call; add a useEffect cleanup that on unmount checks
collapseTimer.current, calls clearTimeout if present, and sets
collapseTimer.current = null so any pending timeout started by
handleExpandedChange (and using COLLAPSE_ANIMATION_MS) is cancelled when the
component unmounts.
In `@apps/mobile/screens/`(authenticated)/components/OrgDropdown/OrgDropdown.tsx:
- Around line 30-54: OrgDropdown duplicates session/orgs/finding/switching
logic; refactor to reuse the existing useOrganizations hook by removing local
session/orgs/find/switch logic and consuming the hook's data and actions
instead. Update the useOrganizations hook (in useOrganizations.ts) to expose the
switching state (isSwitching) and an action like switchOrganization or
setActiveOrganization that encapsulates the current handleSwitchOrg behavior
(including router.replace). In OrgDropdown.tsx, replace local state "switching"
and the handleSwitchOrg function with the hook's isSwitching and
switchOrganization API and use the hook's org list and activeOrg lookup (keep
orgInitial calculation if needed). Ensure callers such as WorkspacesScreen and
TabBarAccessory continue to work with the expanded hook surface.
In
`@apps/mobile/screens/`(authenticated)/hooks/useOrganizations/useOrganizations.ts:
- Around line 10-11: The code currently assigns authClient.useSession() to a
session variable and accesses nested fields
(session.data?.session?.activeOrganizationId); change this to use the same
destructuring pattern used elsewhere by calling authClient.useSession() and
destructuring its returned shape (e.g., { data, ... } or { data: { session } }
as needed) so you can directly access activeOrganizationId without extra
nesting; update references in this file (the useSession() call and any use of
session.data?.session?.activeOrganizationId) to the destructured names so it
matches CollectionsProvider/RootLayout/PostHogUserIdentifier patterns.
🧹 Nitpick comments (27)
apps/mobile/screens/RootLayout/RootLayout.tsx (1)
9-9: Module-level side effect for theme initialization.
Uniwind.setTheme("dark")executes on import, which means it runs every time this module is loaded. This is likely intentional to guarantee the dark theme is set before any component renders, but be aware this makes light-mode support harder to add later and couples theme selection to module loading order.apps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.tsx (2)
16-22: Consider addingaccessibilityLabelto the avatar image.When rendering the logo, adding an
accessibilityLabel(e.g., the organization name) would improve screen reader support.Suggested improvement
return ( <Image source={{ uri: logo }} style={{ width: size, height: size, borderRadius: size / 2 }} + accessibilityLabel={name ?? "Organization logo"} /> );
25-25: Nit: extract the default initial as a named constant.The fallback
"O"is a magic string. A named constant likeDEFAULT_ORG_INITIALwould make intent clearer. As per coding guidelines: "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic."apps/mobile/screens/(auth)/sign-in/components/SocialButton/SocialButton.tsx (1)
58-59:useColorScheme()can returnnull.On some platforms/initial renders,
useColorScheme()returnsnull. The current code handles this safely (falls through to"black"), but an explicit fallback makes the intent clearer and guards against future refactors.Optional: explicit fallback
const colorScheme = useColorScheme(); -const iconColor = colorScheme === "dark" ? "white" : "black"; +const iconColor = colorScheme === "dark" ? "white" : "black"; // null → "black" (light default)A comment is the lightest option. Alternatively:
const iconColor = colorScheme === "dark" ? "white" : "black";is already correct — just flagging for awareness.packages/ui/package.json (1)
89-91: Consider using a version range for thereactpeer dependency.The
reactpeer dependency is pinned to an exact version (19.2.0). For a shared UI package consumed across the monorepo, this is fragile — any consumer on a patch like19.2.1would trigger peer dependency warnings. A semver range like^19.0.0(similar to howpackages/durable-sessiondeclares"^18.0.0 || ^19.0.0") would be more resilient to minor/patch bumps.Suggested change
"peerDependencies": { - "react": "19.2.0" + "react": "^19.0.0" }apps/mobile/screens/(authenticated)/(tasks)/tasks/TasksScreen.tsx (1)
8-12: Refresh handler completes synchronously — spinner won't be visible.Since there's no
awaitbetweensetRefreshing(true)andsetRefreshing(false), React will batch both state updates and the refresh indicator will never render. When the actual data fetch is added, ensure theawaitprecedessetRefreshing(false).const onRefresh = useCallback(async () => { setRefreshing(true); - // TODO: refresh task data + // TODO: refresh task data — await the async work here + // await fetchTasks(); setRefreshing(false); }, []);Would you like me to open an issue to track implementing the task data refresh?
apps/mobile/screens/(authenticated)/tasks/[id]/TaskDetailScreen.tsx (2)
19-24: Consider extracting the shared back-button header into a reusable component.This exact header pattern (back chevron + title in a flex-row) is duplicated in
SettingsScreen.tsxandWorkspaceDetailScreen.tsx. As more screens are added, this will proliferate. A smallScreenHeadercomponent under@/components/ui/would DRY this up.// Example: `@/components/ui/screen-header.tsx` export function ScreenHeader({ title, onBack }: { title: string; onBack: () => void }) { return ( <View className="flex-row items-center gap-2"> <Pressable onPress={onBack} className="p-1"> <Icon as={ChevronLeft} className="text-foreground size-6" /> </Pressable> <Text className="text-2xl font-bold">{title}</Text> </View> ); }
13-17: ScrollView with safe-area pattern is also repeated across screens.The
<ScrollView className="flex-1 bg-background" contentContainerStyle={{ paddingTop: insets.top }}>wrapper is identical in all three new screens. Consider combining this with the header into aScreenLayoutwrapper to reduce boilerplate.apps/mobile/modules/tab-bar/ios/TabBar.podspec (1)
24-24: Broad source_files glob may pick up unintended files.
"**/*.{h,m,mm,swift,hpp,cpp}"recursively matches all subdirectories. If any test or example files land under this path, they'll be compiled into the pod. Consider scoping to a specific directory or excluding test paths.apps/mobile/modules/tab-bar/ios/CollapsedBarView.swift (1)
11-11: Unnecessaryenumerated()— index is unused.Since the tuple index is discarded (
_), you can simplify toForEach(tabs, id: \.name)directly.Proposed fix
- ForEach(Array(tabs.enumerated()), id: \.element.name) { _, tab in + ForEach(tabs, id: \.name) { tab inapps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/OrganizationSwitcherSheet.tsx (1)
86-86: Extract hardcoded domain string to a named constant.
"superset.sh/"is a magic string. If the domain changes, every occurrence must be found manually.Proposed fix
+const ORG_SLUG_PREFIX = "superset.sh/"; // ... - superset.sh/{organization.slug} + {ORG_SLUG_PREFIX}{organization.slug}As per coding guidelines, "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic."
apps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/index.ts (1)
1-1: Consider re-exporting theOrganizationtype.
OrganizationSwitcherSheet.tsxexports theOrganizationinterface, but this barrel doesn't re-export it. Consumers needing the type would have to bypass the barrel and import directly from the implementation file.Proposed fix
-export { OrganizationSwitcherSheet } from "./OrganizationSwitcherSheet"; +export { OrganizationSwitcherSheet, type Organization } from "./OrganizationSwitcherSheet";apps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationHeaderButton/OrganizationHeaderButton.tsx (2)
25-25: Extract the hardcoded color to a named constant or use a theme value.
"hsl(240 5% 64.9%)"is a magic string. Given thatOrganizationAvataralready usesuseTheme(), consider using a theme color (e.g.,theme.mutedForeground) for consistency, or at minimum extract this to a named constant.♻️ Proposed fix
+const CHEVRON_COLOR = "hsl(240 5% 64.9%)"; + export function OrganizationHeaderButton({- <ChevronsUpDown size={14} color="hsl(240 5% 64.9%)" /> + <ChevronsUpDown size={14} color={CHEVRON_COLOR} />Or better, use a theme token if one matches this color.
As per coding guidelines: "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic."
29-31: No-op edit button — is this intentional?The right toolbar button has an empty
onPress={() => {}}. If this is a placeholder for future functionality, consider adding a brief comment or TODO to clarify intent.apps/mobile/package.json (2)
6-6:EXPO_UNSTABLE_MCP_SERVER=1in dev script.This uses an unstable/experimental Expo feature. Worth adding a brief comment in the script or README noting why this is enabled, so future developers understand the dependency on this flag.
57-73: Multiple beta/preview dependencies — ensure the team is tracking stability.Several core dependencies are on pre-release versions (
expo@^55.0.0-beta,expo-router@~55.0.0-preview.6,react-native-gesture-handler@3.0.0-beta.1). As of mid-February 2026, Expo SDK 55 remains in beta with no stable release announced yet, so these versions may change before final release. Consider pinning to exact versions to avoid unexpected breakage from pre-release semver ranges.apps/mobile/modules/tab-bar/ios/GlassCircleButton.swift (1)
7-14: Consider adding an accessibility label.The button relies on the SF Symbol name for VoiceOver, which may not produce a user-friendly description for all icons (e.g.,
"magnifyingglass"reads as "magnifyingglass"). Adding an explicitaccessibilityLabelparameter would improve the VoiceOver experience.♻️ Suggested improvement
struct GlassCircleButton: View { let icon: String + var accessibilityLabel: String? = nil let action: () -> Void var body: some View { Button(action: action) { Image(systemName: icon) .font(.system(size: 16, weight: .medium)) .foregroundStyle(.white) .frame(width: 44, height: 44) .background(.ultraThinMaterial, in: Circle()) } + .accessibilityLabel(accessibilityLabel ?? icon) } }apps/mobile/modules/tab-bar/src/TabBarView.tsx (1)
29-46: Consider extracting inline event handler type annotations.The inline
NativeSyntheticEvent<{ name: string }>annotations on Lines 33, 38, and 43 duplicate the types already declared inNativeTabBarViewProps. They're harmless but redundant — the compiler already infers the parameter types from theNativeViewprop types.apps/mobile/screens/(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.tsx (1)
62-67: Placeholder handlers are fine for now, but consider logging intent.
onMenuActionPressandonSearchPresssilently discard events. A// TODOcomment is present foronMenuActionPressbut not foronSearchPress. Adding a briefconsole.debug("[tab-bar] search pressed — not yet implemented")could help during development. Optional nit.apps/mobile/modules/tab-bar/ios/TabBarView.swift (2)
19-74: Extract the repeated spring animation to a constant.The same
.spring(response: 0.35, dampingFraction: 0.85)appears 6 times. Extracting it reduces duplication and makes tuning easier.♻️ Suggested improvement
Add near the top of the file or as a private property:
private let menuSpring = Animation.spring(response: 0.35, dampingFraction: 0.85)Then replace all occurrences:
- withAnimation(.spring(response: 0.35, dampingFraction: 0.85)) { + withAnimation(menuSpring) {
92-94: RefactoronChangeclosure for iOS 17+ compatibility.The 1-parameter
onChange(of:) { newValue in }form is deprecated in iOS 17. Update to the 2-parameter form:Suggested change
.onChange(of: isExpanded) { newValue in - props.onExpandedChange(["expanded": newValue]) + props.onExpandedChange(["expanded": $1]) }Or refactor to:
.onChange(of: isExpanded) { oldValue, newValue in props.onExpandedChange(["expanded": newValue]) }Since Expo SDK 55 targets iOS 15.1+, no warnings appear now, but this will be needed when deployment target advances to iOS 17+.
apps/mobile/screens/(authenticated)/hooks/useOrganizations/useOrganizations.ts (1)
22-33: WrapswitchOrganizationinuseCallbackto stabilize identity.
switchOrganizationis recreated every render. Consumers that pass it as a prop (e.g.,WorkspacesScreen) will trigger unnecessary child re-renders. Since it capturesactiveOrganizationIdandrouter, both should be in the dependency array.♻️ Suggested improvement
- const switchOrganization = async (organizationId: string) => { + const switchOrganization = useCallback(async (organizationId: string) => { if (organizationId === activeOrganizationId) return; try { await authClient.organization.setActive({ organizationId }); router.replace("/(authenticated)/(home)"); } catch (error) { console.error( "[organization/switch] Failed to switch organization:", error, ); } - }; + }, [activeOrganizationId, router]);Add
useCallbackto the import:import { useCallback } from "react";apps/mobile/screens/(authenticated)/(home)/workspaces/WorkspacesScreen.tsx (1)
24-27:switchOrganizationis async but not awaited — the sheet closes before the switch completes.
switchOrganizationis anasyncfunction (seeuseOrganizationshook). Not awaiting it here means the sheet closes optimistically regardless of whether the switch succeeds or fails. If the switch fails (network error), the user sees the sheet close with no feedback. Consider awaiting and handling failure, e.g. by re-opening the sheet or showing an error.Proposed fix
-const handleSwitchOrganization = (organizationId: string) => { +const handleSwitchOrganization = async (organizationId: string) => { setSheetOpen(false); - switchOrganization(organizationId); + await switchOrganization(organizationId); };apps/mobile/screens/(authenticated)/components/OrgDropdown/OrgDropdown.tsx (1)
85-98: Duplicate filter — extractotherOrgsto avoid computing it twice.The same
orgs?.filter((org) => org.id !== activeOrgId)is evaluated on line 85 for the list and again on line 97 for the separator conditional.Proposed fix
+ const otherOrgs = orgs?.filter((org) => org.id !== activeOrgId) ?? []; ... - {orgs - ?.filter((org) => org.id !== activeOrgId) - .map((org) => ( + {otherOrgs.map((org) => ( ... ))} - {orgs && orgs.filter((org) => org.id !== activeOrgId).length > 0 && ( + {otherOrgs.length > 0 && ( <DropdownMenuSeparator /> )}apps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.tsx (2)
8-9: Deep import into another component's internals — consider promotingOrganizationAvatar.
OrganizationAvataris imported fromOrganizationSwitcherSheet/components/, which couples this component to the internal structure ofOrganizationSwitcherSheet. SinceOrganizationAvataris now used by multiple components (here and inside the sheet), consider promoting it to a shared location (e.g.,screens/(authenticated)/components/or a common UI folder).
24-27:switchOrganizationis async but not awaited here either.Same concern as
WorkspacesScreen— the sheet closes optimistically without waiting for the switch result. Consider awaiting for consistency and error feedback.apps/mobile/modules/tab-bar/ios/ExpandedMenuView.swift (1)
42-42: SimplifyForEach—.enumerated()is unnecessary since the index is discarded.Both
ForEachcalls wrap arrays inArray(....enumerated())but discard the index (_). This can be simplified to idiomatic SwiftUI:Proposed simplification
-ForEach(Array(tabs.filter { !$0.isMenuTrigger }.enumerated()), id: \.element.name) { _, tab in +ForEach(tabs.filter { !$0.isMenuTrigger }, id: \.name) { tab in-ForEach(Array(menuActions.enumerated()), id: \.element.name) { _, action in +ForEach(menuActions, id: \.name) { action inAlso applies to: 77-77
| Divider() | ||
| .background(Color.white.opacity(0.2)) |
There was a problem hiding this comment.
.background on Divider may not produce the intended color effect.
SwiftUI's Divider renders its own line; applying .background(Color.white.opacity(0.2)) sets the background behind the divider, not the divider's line color. To tint the divider itself, use .overlay or replace it with a styled Rectangle.
Proposed fix using overlay or Rectangle
-Divider()
- .background(Color.white.opacity(0.2))
- .padding(.vertical, 4)
+Rectangle()
+ .fill(Color.white.opacity(0.2))
+ .frame(height: 1 / UIScreen.main.scale)
+ .padding(.vertical, 4)🤖 Prompt for AI Agents
In `@apps/mobile/modules/tab-bar/ios/ExpandedMenuView.swift` around lines 73 - 74,
The Divider's .background modifier won't tint the divider line; replace the
current Divider().background(Color.white.opacity(0.2)) with a divider-styling
that actually colors the line—either use an overlay (e.g.,
Divider().overlay(Rectangle().foregroundColor(Color.white.opacity(0.2))).frame(height:
1)) or swap the Divider for a Rectangle directly
(Rectangle().foregroundColor(Color.white.opacity(0.2)).frame(height: 1)); update
the code in ExpandedMenuView where Divider is used to one of these options so
the visible line color matches the intended semi-transparent white.
| s.homepage = 'https://superset.sh' | ||
| s.platforms = { :ios => '15.1' } | ||
| s.swift_version = '5.9' | ||
| s.source = { git: 'https://github.com/nicksupersetsh/superset.git' } |
There was a problem hiding this comment.
Source URL points to a personal fork, not the organization repo.
s.source references nicksupersetsh/superset.git but the project lives at superset-sh/superset. This will cause issues if the pod is ever resolved from source.
Proposed fix
- s.source = { git: 'https://github.com/nicksupersetsh/superset.git' }
+ s.source = { git: 'https://github.com/superset-sh/superset.git' }📝 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.
| s.source = { git: 'https://github.com/nicksupersetsh/superset.git' } | |
| s.source = { git: 'https://github.com/superset-sh/superset.git' } |
🤖 Prompt for AI Agents
In `@apps/mobile/modules/tab-bar/ios/TabBar.podspec` at line 15, The Podspec's
s.source is pointing to the personal fork (nicksupersetsh/superset.git); update
the s.source value in TabBar.podspec to point to the canonical repository
(superset-sh/superset.git) so the pod resolves from the organization repo
(change the git URL assigned to s.source accordingly).
| const TERMS_URL = "https://superset.sh/terms"; | ||
| const PRIVACY_URL = "https://superset.sh/privacy"; |
There was a problem hiding this comment.
Unhandled promise rejection from Linking.openURL.
Linking.openURL returns a Promise that can reject (e.g., if no handler is installed for the URL scheme). The onPress callbacks on lines 67 and 74 fire-and-forget without a .catch.
Proposed fix
- <Text
- className="text-xs text-muted-foreground underline"
- onPress={() => Linking.openURL(TERMS_URL)}
- >
+ <Text
+ className="text-xs text-muted-foreground underline"
+ onPress={() => Linking.openURL(TERMS_URL).catch(() => {})}
+ >
Terms of Service
</Text>{" "}
and{" "}
<Text
- className="text-xs text-muted-foreground underline"
- onPress={() => Linking.openURL(PRIVACY_URL)}
+ className="text-xs text-muted-foreground underline"
+ onPress={() => Linking.openURL(PRIVACY_URL).catch(() => {})}
>Or better yet, extract a helper that logs on failure per the coding guideline on never swallowing errors silently:
const openURL = (url: string) =>
Linking.openURL(url).catch((err) =>
console.error("[sign-in] Failed to open URL:", url, err),
);Also applies to: 63-78
🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(auth)/sign-in/SignInScreen.tsx around lines 9 - 10, The
onPress handlers that call Linking.openURL with TERMS_URL and PRIVACY_URL can
reject and currently fire-and-forget; add a small helper (e.g., openURL) that
calls Linking.openURL(url).catch(err => console.error("[sign-in] Failed to open
URL:", url, err)) and replace direct Linking.openURL calls in the SignInScreen
onPress callbacks with this helper to ensure promise rejections are logged
instead of swallowed.
| const onRefresh = useCallback(async () => { | ||
| setRefreshing(true); | ||
| setRefreshing(false); | ||
| }, []); |
There was a problem hiding this comment.
onRefresh is a no-op — the spinner will flash and immediately disappear.
setRefreshing(true) followed immediately by setRefreshing(false) with no awaited work in between means the pull-to-refresh indicator won't persist long enough to be visible, and no data is actually refreshed. If this is intentional scaffolding, consider adding a TODO comment; otherwise wire it to an actual data refetch.
Proposed placeholder fix
const onRefresh = useCallback(async () => {
setRefreshing(true);
+ // TODO: trigger actual data refresh here, e.g.:
+ // await refetchWorkspaces();
setRefreshing(false);
}, []);📝 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.
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| setRefreshing(false); | |
| }, []); | |
| const onRefresh = useCallback(async () => { | |
| setRefreshing(true); | |
| // TODO: trigger actual data refresh here, e.g.: | |
| // await refetchWorkspaces(); | |
| setRefreshing(false); | |
| }, []); |
🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(authenticated)/(home)/workspaces/WorkspacesScreen.tsx
around lines 29 - 32, onRefresh is currently a no-op because it sets
setRefreshing(true) then immediately setRefreshing(false); update useCallback
onRefresh to perform the actual refresh by invoking the workspace data
loader/refetch function (e.g., call loadWorkspaces, fetchWorkspaces, or
refetchWorkspaces) and await its result, and ensure setRefreshing(false) runs in
a finally block so the spinner stops after the fetch completes; if this is
intentional scaffolding, replace the body with a clear TODO comment referencing
onRefresh and setRefreshing to indicate the refresh implementation is pending.
| const [switching, setSwitching] = useState(false); | ||
|
|
||
| const session = authClient.useSession(); | ||
| const activeOrgId = session.data?.session?.activeOrganizationId; | ||
|
|
||
| const { data: orgs } = useLiveQuery( | ||
| (q) => q.from({ organizations: collections.organizations }), | ||
| [collections], | ||
| ); | ||
|
|
||
| const activeOrg = orgs?.find((org) => org.id === activeOrgId); | ||
| const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?"; | ||
| const otherOrgs = orgs?.filter((org) => org.id !== activeOrgId) ?? []; | ||
|
|
||
| const handleSwitchOrg = async (orgId: string) => { | ||
| if (orgId === activeOrgId) return; | ||
| setSwitching(true); | ||
| try { | ||
| await authClient.organization.setActive({ organizationId: orgId }); | ||
| router.replace("/(authenticated)/(home)"); | ||
| } catch (error) { | ||
| console.error("[org/switch] Failed to switch organization:", error); | ||
| } finally { | ||
| setSwitching(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Same duplicated org-switching logic — reuse useOrganizations hook.
This is the same duplication flagged in OrgDropdown.tsx. Lines 27–50 reimplement session fetching, org querying, active org resolution, and switch handling that useOrganizations already provides. The only addition is the switching loading state — consider adding that to the shared hook so both OrgDropdown and MoreMenuScreen can consume it.
Proposed refactor sketch
-import { useLiveQuery } from "@tanstack/react-db";
...
-import { authClient } from "@/lib/auth/client";
-import { useCollections } from "@/screens/(authenticated)/providers/CollectionsProvider";
+import { useOrganizations } from "@/screens/(authenticated)/hooks/useOrganizations";
export function MoreMenuScreen() {
const router = useRouter();
const insets = useSafeAreaInsets();
const { signOut } = useSignOut();
- const collections = useCollections();
- const [switching, setSwitching] = useState(false);
-
- const session = authClient.useSession();
- const activeOrgId = session.data?.session?.activeOrganizationId;
-
- const { data: orgs } = useLiveQuery(
- (q) => q.from({ organizations: collections.organizations }),
- [collections],
- );
-
- const activeOrg = orgs?.find((org) => org.id === activeOrgId);
- const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?";
- const otherOrgs = orgs?.filter((org) => org.id !== activeOrgId) ?? [];
-
- const handleSwitchOrg = async (orgId: string) => {
- if (orgId === activeOrgId) return;
- setSwitching(true);
- try {
- await authClient.organization.setActive({ organizationId: orgId });
- router.replace("/(authenticated)/(home)");
- } catch (error) {
- console.error("[org/switch] Failed to switch organization:", error);
- } finally {
- setSwitching(false);
- }
- };
+ const {
+ organizations: orgs,
+ activeOrganization: activeOrg,
+ activeOrganizationId: activeOrgId,
+ switchOrganization,
+ } = useOrganizations();
+
+ const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?";
+ const otherOrgs = orgs?.filter((org) => org.id !== activeOrgId) ?? [];🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(authenticated)/(more)/MoreMenuScreen.tsx around lines
25 - 50, MoreMenuScreen duplicates organization logic (session, orgs query,
activeOrg resolution, handleSwitchOrg and switching state) that the shared
useOrganizations hook already encapsulates; refactor MoreMenuScreen to import
and use useOrganizations instead of re-implementing that logic, move the local
switching state into the shared hook (or expose a switching setter and switchOrg
method) so MoreMenuScreen can call the hook's switch function (replace
handleSwitchOrg), consume activeOrg, otherOrgs, and switching from the hook, and
remove the duplicate useLiveQuery/session code—ensure the hook API matches what
OrgDropdown expects so both components reuse the same functions and state.
| const handleExpandedChange = useCallback((expanded: boolean) => { | ||
| if (expanded) { | ||
| // Expand container immediately so SwiftUI has room to animate into | ||
| if (collapseTimer.current) { | ||
| clearTimeout(collapseTimer.current); | ||
| collapseTimer.current = null; | ||
| } | ||
| setIsExpanded(true); | ||
| } else { | ||
| // Delay container shrink so the SwiftUI close animation isn't clipped | ||
| collapseTimer.current = setTimeout(() => { | ||
| setIsExpanded(false); | ||
| collapseTimer.current = null; | ||
| }, COLLAPSE_ANIMATION_MS); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Missing cleanup of collapseTimer on unmount.
If the component unmounts while a collapse timeout is pending (e.g., user navigates away mid-animation), the timer fires and calls setIsExpanded on a stale closure. While React 19 won't crash, it's still a leaked timer. Add a cleanup effect.
🛡️ Proposed fix
Add after the handleExpandedChange declaration (e.g., after Line 49):
import { useCallback, useEffect, useRef, useState } from "react"; }, []);
+
+ useEffect(() => {
+ return () => {
+ if (collapseTimer.current) {
+ clearTimeout(collapseTimer.current);
+ }
+ };
+ }, []);
return (📝 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.
| const handleExpandedChange = useCallback((expanded: boolean) => { | |
| if (expanded) { | |
| // Expand container immediately so SwiftUI has room to animate into | |
| if (collapseTimer.current) { | |
| clearTimeout(collapseTimer.current); | |
| collapseTimer.current = null; | |
| } | |
| setIsExpanded(true); | |
| } else { | |
| // Delay container shrink so the SwiftUI close animation isn't clipped | |
| collapseTimer.current = setTimeout(() => { | |
| setIsExpanded(false); | |
| collapseTimer.current = null; | |
| }, COLLAPSE_ANIMATION_MS); | |
| } | |
| }, []); | |
| const handleExpandedChange = useCallback((expanded: boolean) => { | |
| if (expanded) { | |
| // Expand container immediately so SwiftUI has room to animate into | |
| if (collapseTimer.current) { | |
| clearTimeout(collapseTimer.current); | |
| collapseTimer.current = null; | |
| } | |
| setIsExpanded(true); | |
| } else { | |
| // Delay container shrink so the SwiftUI close animation isn't clipped | |
| collapseTimer.current = setTimeout(() => { | |
| setIsExpanded(false); | |
| collapseTimer.current = null; | |
| }, COLLAPSE_ANIMATION_MS); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| return () => { | |
| if (collapseTimer.current) { | |
| clearTimeout(collapseTimer.current); | |
| } | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In
`@apps/mobile/screens/`(authenticated)/components/AuthenticatedTabBar/AuthenticatedTabBar.tsx
around lines 34 - 49, The collapse timeout (collapseTimer used inside
handleExpandedChange) is not cleared on unmount, risking a leaked timer and a
stale setIsExpanded call; add a useEffect cleanup that on unmount checks
collapseTimer.current, calls clearTimeout if present, and sets
collapseTimer.current = null so any pending timeout started by
handleExpandedChange (and using COLLAPSE_ANIMATION_MS) is cancelled when the
component unmounts.
| const [switching, setSwitching] = useState(false); | ||
|
|
||
| const session = authClient.useSession(); | ||
| const activeOrgId = session.data?.session?.activeOrganizationId; | ||
|
|
||
| const { data: orgs } = useLiveQuery( | ||
| (q) => q.from({ organizations: collections.organizations }), | ||
| [collections], | ||
| ); | ||
|
|
||
| const activeOrg = orgs?.find((org) => org.id === activeOrgId); | ||
| const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?"; | ||
|
|
||
| const handleSwitchOrg = async (orgId: string) => { | ||
| if (orgId === activeOrgId) return; | ||
| setSwitching(true); | ||
| try { | ||
| await authClient.organization.setActive({ organizationId: orgId }); | ||
| router.replace("/(authenticated)/(home)"); | ||
| } catch (error) { | ||
| console.error("[org/switch] Failed to switch organization:", error); | ||
| } finally { | ||
| setSwitching(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicated org-fetching and switching logic — reuse useOrganizations hook.
Lines 32–54 reimplement the same session/query/find/switch logic already encapsulated in useOrganizations (see apps/mobile/screens/(authenticated)/hooks/useOrganizations/useOrganizations.ts). The only addition here is the switching loading state. Consider extending useOrganizations to expose an isSwitching flag and reuse it here, as WorkspacesScreen and TabBarAccessory already do.
Proposed refactor sketch
-import { useLiveQuery } from "@tanstack/react-db";
-import { useRouter } from "expo-router";
+import { useRouter } from "expo-router";
...
-import { authClient } from "@/lib/auth/client";
-import { useCollections } from "@/screens/(authenticated)/providers/CollectionsProvider";
+import { useOrganizations } from "@/screens/(authenticated)/hooks/useOrganizations";
export function OrgDropdown() {
const router = useRouter();
const { signOut } = useSignOut();
- const collections = useCollections();
- const [switching, setSwitching] = useState(false);
-
- const session = authClient.useSession();
- const activeOrgId = session.data?.session?.activeOrganizationId;
-
- const { data: orgs } = useLiveQuery(
- (q) => q.from({ organizations: collections.organizations }),
- [collections],
- );
-
- const activeOrg = orgs?.find((org) => org.id === activeOrgId);
- const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?";
-
- const handleSwitchOrg = async (orgId: string) => {
- if (orgId === activeOrgId) return;
- setSwitching(true);
- try {
- await authClient.organization.setActive({ organizationId: orgId });
- router.replace("/(authenticated)/(home)");
- } catch (error) {
- console.error("[org/switch] Failed to switch organization:", error);
- } finally {
- setSwitching(false);
- }
- };
+ const {
+ organizations: orgs,
+ activeOrganization: activeOrg,
+ activeOrganizationId: activeOrgId,
+ switchOrganization,
+ } = useOrganizations();
+
+ const orgInitial = activeOrg?.name?.charAt(0).toUpperCase() ?? "?";🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(authenticated)/components/OrgDropdown/OrgDropdown.tsx
around lines 30 - 54, OrgDropdown duplicates session/orgs/finding/switching
logic; refactor to reuse the existing useOrganizations hook by removing local
session/orgs/find/switch logic and consuming the hook's data and actions
instead. Update the useOrganizations hook (in useOrganizations.ts) to expose the
switching state (isSwitching) and an action like switchOrganization or
setActiveOrganization that encapsulates the current handleSwitchOrg behavior
(including router.replace). In OrgDropdown.tsx, replace local state "switching"
and the handleSwitchOrg function with the hook's isSwitching and
switchOrganization API and use the hook's org list and activeOrg lookup (keep
orgInitial calculation if needed). Ensure callers such as WorkspacesScreen and
TabBarAccessory continue to work with the expanded hook surface.
| const session = authClient.useSession(); | ||
| const activeOrganizationId = session.data?.session?.activeOrganizationId; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for authClient.useSession definition and usage
rg -n "useSession" --type=ts -C5 | head -100Repository: superset-sh/superset
Length of output: 10456
🏁 Script executed:
# Find authClient definition
rg -n "authClient" --type=ts -C3 | head -150Repository: superset-sh/superset
Length of output: 13237
🏁 Script executed:
# Look for the useOrganizations file specifically
find . -name "useOrganizations.ts" -type fRepository: superset-sh/superset
Length of output: 144
🏁 Script executed:
# Find authClient definition
find . -path "*/lib/auth/client*" -type fRepository: superset-sh/superset
Length of output: 96
🏁 Script executed:
# Search for useSession hook definition in the auth module
rg -n "export.*useSession" --type=ts -C10Repository: superset-sh/superset
Length of output: 647
🏁 Script executed:
# Read the full authClient file to understand the useSession return type
cat -n apps/mobile/lib/auth/client.tsRepository: superset-sh/superset
Length of output: 742
🏁 Script executed:
# Also check if there's a shared auth client package
find . -path "*/auth/client*" -type f | grep -v node_modules | head -20Repository: superset-sh/superset
Length of output: 96
🏁 Script executed:
# Search for better-auth types or documentation references
rg -n "better-auth" --type=ts -C2 | head -50Repository: superset-sh/superset
Length of output: 3245
🏁 Script executed:
# Check how useSession is used in other files to understand the return type
rg -n "useSession\(\)" --type=ts -A3 | head -100Repository: superset-sh/superset
Length of output: 10627
🏁 Script executed:
# Count destructuring patterns in mobile app to determine standard
rg "const.*session.*authClient\.useSession\(\)" apps/mobile --type=ts -A1 | grep -E "const.*session|const.*{.*data.*session" | wc -lRepository: superset-sh/superset
Length of output: 65
🏁 Script executed:
# Show all patterns side-by-side
rg "const.*session.*useSession\(\)" apps/mobile --type=ts -B1 | head -40Repository: superset-sh/superset
Length of output: 1571
🏁 Script executed:
# Verify the exact imports used - some use useSession from authClient, some import useSession
rg "import.*useSession|from.*useSession" apps/mobile --type=tsRepository: superset-sh/superset
Length of output: 538
🏁 Script executed:
# Show the full context of how useOrganizations.ts uses the session vs other files
cat -n apps/mobile/screens/\(authenticated\)/hooks/useOrganizations/useOrganizations.tsRepository: superset-sh/superset
Length of output: 1518
🏁 Script executed:
# Compare with a file that uses destructuring pattern
cat -n apps/mobile/screens/\(authenticated\)/providers/CollectionsProvider/CollectionsProvider.tsx | head -25Repository: superset-sh/superset
Length of output: 1077
Use destructuring pattern for useSession() to match other mobile files.
useOrganizations.ts should destructure the session data like CollectionsProvider, RootLayout, and PostHogUserIdentifier do:
- const session = authClient.useSession();
- const activeOrganizationId = session.data?.session?.activeOrganizationId;
+ const { data: session } = authClient.useSession();
+ const activeOrganizationId = session?.session?.activeOrganizationId;This reduces nesting and aligns with the established pattern across the mobile app.
📝 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.
| const session = authClient.useSession(); | |
| const activeOrganizationId = session.data?.session?.activeOrganizationId; | |
| const { data: session } = authClient.useSession(); | |
| const activeOrganizationId = session?.session?.activeOrganizationId; |
🤖 Prompt for AI Agents
In
`@apps/mobile/screens/`(authenticated)/hooks/useOrganizations/useOrganizations.ts
around lines 10 - 11, The code currently assigns authClient.useSession() to a
session variable and accesses nested fields
(session.data?.session?.activeOrganizationId); change this to use the same
destructuring pattern used elsewhere by calling authClient.useSession() and
destructuring its returned shape (e.g., { data, ... } or { data: { session } }
as needed) so you can directly access activeOrganizationId without extra
nesting; update references in this file (the useSession() call and any use of
session.data?.session?.activeOrganizationId) to the destructured names so it
matches CollectionsProvider/RootLayout/PostHogUserIdentifier patterns.
saddlepaddle
changed the title
Replace NativeTabs with a custom floating tab bar built as a local Expo Module using SwiftUI. The tab bar features a Linear-style glass capsule with expand/collapse spring animation for the navigation menu. - New `@superset/tab-bar` local Expo Module (SwiftUI + ExpoModulesCore) - Collapsed: glass capsule with Home/Tasks/More icons + search circle - Expanded: org switcher pill, settings gear, menu items with spring animation - Headless Tabs (expo-router/ui) for routing with hidden TabList - AuthenticatedTabBar bridges native SwiftUI view to expo-router - Dynamic container resizing (96px collapsed, full-screen expanded) via onExpandedChange event from SwiftUI to JS
saddlepaddle
force-pushed
the
implement-mobie
branch
from
afab44e to
68db430
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@apps/mobile/screens/`(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.tsx:
- Around line 16-22: The OrganizationAvatar component currently renders an
<Image> when the logo prop is present but has no fallback if the image fails to
load; add a local boolean state (e.g., imageError via useState) and render the
<Image> only when logo is present and imageError is false, attaching an onError
handler on the <Image> to set imageError true; when imageError is true (or logo
is absent) fall back to the existing initials-based avatar rendering so
broken/stale logo URLs show the initial avatar instead.
In `@apps/mobile/screens/`(authenticated)/(more)/settings/SettingsScreen.tsx:
- Around line 20-22: Add proper accessibility attributes to the back Pressable
in SettingsScreen.tsx: update the Pressable (the wrapper around Icon/ChevronLeft
that calls router.back()) to include accessibilityRole="button" and an
accessibilityLabel describing its action (e.g., "Back" or a localized string
like t('back')) so screen readers announce its purpose; ensure the label is
concise and matches any existing localization function if used in this screen.
In `@apps/mobile/screens/RootLayout/RootLayout.tsx`:
- Around line 17-19: The RootLayout currently returns null while the auth
session resolves (useSession -> isPending), which can flash a blank screen;
instead either keep the splash visible until resolution or render a loading
indicator: update RootLayout so that when isPending is true you call your
splash-screen keep-alive (e.g., ensure SplashScreen.preventAutoHideAsync() is
invoked earlier and only hide after session is settled) or replace the early
return null with a stable loading UI (e.g., an ActivityIndicator or a simple
centered loader) so the UI doesn’t go blank while useSession (data: session,
isPending) resolves.
🧹 Nitpick comments (9)
apps/mobile/screens/(authenticated)/tasks/[id]/TaskDetailScreen.tsx (1)
20-22: Consider adding accessibility props to the back button.The
PressablelacksaccessibilityLabelandaccessibilityRole, which hurts VoiceOver/TalkBack usability.♿ Proposed fix
- <Pressable onPress={() => router.back()} className="p-1"> + <Pressable + onPress={() => router.back()} + className="p-1" + accessibilityRole="button" + accessibilityLabel="Go back" + >apps/mobile/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.tsx (1)
25-38: Extract inline magic values to named constants.
0.45(font-size ratio) and"O"(default initial) are magic values. As per coding guidelines, extract hardcoded magic numbers and strings to named constants at module top.Suggested extraction
+const FONT_SIZE_RATIO = 0.45; +const DEFAULT_INITIAL = "O"; + export function OrganizationAvatar({- const initial = (name ?? "O").charAt(0).toUpperCase(); + const initial = (name ?? DEFAULT_INITIAL).charAt(0).toUpperCase();- style={{ fontSize: size * 0.45, color: theme.mutedForeground }} + style={{ fontSize: size * FONT_SIZE_RATIO, color: theme.mutedForeground }}apps/mobile/screens/(authenticated)/(more)/MoreMenuScreen.tsx (2)
83-98: No visual feedback during org switch.
switchingdisables the pressables but there's no spinner or opacity change to indicate the operation is in progress. Users may think the tap didn't register.Suggested approach
<Pressable key={org.id} onPress={() => handleSwitchOrg(org.id)} disabled={switching} - className="flex-row items-center gap-3 px-4 py-3" + className={`flex-row items-center gap-3 px-4 py-3 ${switching ? "opacity-50" : ""}`} >Or render an
ActivityIndicatorwhenswitchingis true.
127-133: Consider adding a confirmation before sign-out.Tapping "Log out" immediately signs the user out with no confirmation step. An accidental tap could be disruptive. A simple
Alert.alertconfirmation would be a low-effort safeguard.apps/mobile/screens/(authenticated)/hooks/useOrganizations/useOrganizations.ts (1)
6-41: Add type annotations for the returned organization data.The hook returns untyped organization data (
any[]/any), which removes type safety for all consumers (WorkspacesScreen,TabBarAccessory,OrganizationSwitcherSheet). Define anOrganizationtype (or re-use the one already exported fromOrganizationSwitcherSheet.tsx) and annotate the return value.Suggested approach
+import type { Organization } from "@/screens/(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet"; + export function useOrganizations() { // ... - const { data: organizations } = useLiveQuery( + const { data: organizations } = useLiveQuery<Organization>( (q) => q.from({ organizations: collections.organizations }), [collections], );As per coding guidelines, "Maintain type safety by avoiding
anytypes unless absolutely necessary."apps/mobile/screens/(authenticated)/components/TabBarAccessory/TabBarAccessory.tsx (2)
8-9: Deep import intoOrganizationSwitcherSheet's internalcomponents/breaks encapsulation.
OrganizationAvataris reached through a sibling component's privatecomponents/directory. If it's needed by multiple components, promote it to a shared location (e.g.,(authenticated)/components/OrganizationAvatar/) or re-export it fromOrganizationSwitcherSheet's barrel.As per coding guidelines, "Nest components under their parent's
components/folder if used once; promote to the highest shared parent'scomponents/if used 2+ times."
12-69: Organization-switching sheet pattern is duplicated withWorkspacesScreen.Both
TabBarAccessoryandWorkspacesScreenmaintain identicalsheetOpenstate,handleSwitchOrganizationcallback, andOrganizationSwitcherSheetrender block. Consider extracting a shared hook (e.g.,useOrganizationSwitcherSheet) that returns{ sheetOpen, openSheet, sheetProps }to DRY this up.apps/mobile/screens/RootLayout/RootLayout.tsx (1)
5-12: Move theUniwind.setThemeside effect after all imports.Placing a side effect call between import statements is fragile — it relies on import ordering and will be flagged by
import/firstor similar lint rules. Move it below the last import.♻️ Proposed fix
import { Uniwind } from "uniwind"; import { useSession } from "@/lib/auth/client"; import { NAV_THEME } from "@/lib/theme"; - -Uniwind.setTheme("dark"); import { PostHogUserIdentifier } from "./components/PostHogUserIdentifier"; import { PostHogProvider } from "./providers/PostHogProvider"; +Uniwind.setTheme("dark"); + const queryClient = new QueryClient();apps/mobile/package.json (1)
6-6:EXPO_UNSTABLE_MCP_SERVERis an unstable/experimental flag.The
EXPO_UNSTABLE_prefix indicates this is an experimental feature. This is fine for development, but be aware that behavior may change or the flag may be renamed/removed in future Expo releases. Ensure theexpo-mcpdevDependency on Line 96 stays in sync if the API evolves.
| if (logo) { | ||
| return ( | ||
| <Image | ||
| source={{ uri: logo }} | ||
| style={{ width: size, height: size, borderRadius: size / 2 }} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
No fallback when the logo image fails to load.
If logo is a stale or broken URL, the Image will silently render as a blank circle. Consider adding an onError callback to fall through to the initial-based avatar.
Proposed approach
+import { useState } from "react";
+
export function OrganizationAvatar({
name,
logo,
size,
}: {
name?: string | null;
logo?: string | null;
size: number;
}) {
const theme = useTheme();
+ const [imageError, setImageError] = useState(false);
- if (logo) {
+ if (logo && !imageError) {
return (
<Image
source={{ uri: logo }}
style={{ width: size, height: size, borderRadius: size / 2 }}
+ onError={() => setImageError(true)}
/>
);
}📝 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.
| if (logo) { | |
| return ( | |
| <Image | |
| source={{ uri: logo }} | |
| style={{ width: size, height: size, borderRadius: size / 2 }} | |
| /> | |
| ); | |
| import { useState } from "react"; | |
| export function OrganizationAvatar({ | |
| name, | |
| logo, | |
| size, | |
| }: { | |
| name?: string | null; | |
| logo?: string | null; | |
| size: number; | |
| }) { | |
| const theme = useTheme(); | |
| const [imageError, setImageError] = useState(false); | |
| if (logo && !imageError) { | |
| return ( | |
| <Image | |
| source={{ uri: logo }} | |
| style={{ width: size, height: size, borderRadius: size / 2 }} | |
| onError={() => setImageError(true)} | |
| /> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
`@apps/mobile/screens/`(authenticated)/(home)/workspaces/components/OrganizationSwitcherSheet/components/OrganizationAvatar/OrganizationAvatar.tsx
around lines 16 - 22, The OrganizationAvatar component currently renders an
<Image> when the logo prop is present but has no fallback if the image fails to
load; add a local boolean state (e.g., imageError via useState) and render the
<Image> only when logo is present and imageError is false, attaching an onError
handler on the <Image> to set imageError true; when imageError is true (or logo
is absent) fall back to the existing initials-based avatar rendering so
broken/stale logo URLs show the initial avatar instead.
| <Pressable onPress={() => router.back()} className="p-1"> | ||
| <Icon as={ChevronLeft} className="text-foreground size-6" /> | ||
| </Pressable> |
There was a problem hiding this comment.
Add an accessibility label to the back button.
The Pressable wrapping the back chevron has no accessibilityLabel or accessibilityRole, which means screen readers won't announce its purpose.
Proposed fix
- <Pressable onPress={() => router.back()} className="p-1">
+ <Pressable
+ onPress={() => router.back()}
+ className="p-1"
+ accessibilityRole="button"
+ accessibilityLabel="Go back"
+ >📝 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.
| <Pressable onPress={() => router.back()} className="p-1"> | |
| <Icon as={ChevronLeft} className="text-foreground size-6" /> | |
| </Pressable> | |
| <Pressable | |
| onPress={() => router.back()} | |
| className="p-1" | |
| accessibilityRole="button" | |
| accessibilityLabel="Go back" | |
| > | |
| <Icon as={ChevronLeft} className="text-foreground size-6" /> | |
| </Pressable> |
🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(authenticated)/(more)/settings/SettingsScreen.tsx
around lines 20 - 22, Add proper accessibility attributes to the back Pressable
in SettingsScreen.tsx: update the Pressable (the wrapper around Icon/ChevronLeft
that calls router.back()) to include accessibilityRole="button" and an
accessibilityLabel describing its action (e.g., "Back" or a localized string
like t('back')) so screen readers announce its purpose; ensure the label is
concise and matches any existing localization function if used in this screen.
| const { data: session, isPending } = useSession(); | ||
|
|
||
| if (isPending) return null; |
There was a problem hiding this comment.
Returning null while session loads may flash a blank screen.
If the Expo splash screen has already been hidden at this point, the user sees an empty view during session resolution. Consider rendering an <ActivityIndicator> or ensuring the splash screen stays visible (e.g., via expo-splash-screen's SplashScreen.preventAutoHideAsync()) until the session state is resolved.
🤖 Prompt for AI Agents
In `@apps/mobile/screens/RootLayout/RootLayout.tsx` around lines 17 - 19, The
RootLayout currently returns null while the auth session resolves (useSession ->
isPending), which can flash a blank screen; instead either keep the splash
visible until resolution or render a loading indicator: update RootLayout so
that when isPending is true you call your splash-screen keep-alive (e.g., ensure
SplashScreen.preventAutoHideAsync() is invoked earlier and only hide after
session is settled) or replace the early return null with a stable loading UI
(e.g., an ActivityIndicator or a simple centered loader) so the UI doesn’t go
blank while useSession (data: session, isPending) resolves.
1 participant
Summary
@superset/tab-bar) built with SwiftUINativeTabswith headlessTabsfromexpo-router/ui+ a native SwiftUI view bridgeArchitecture
Native layer (
modules/tab-bar/ios/):TabBarView.swift— Root SwiftUI view with expand/collapse state machineCollapsedBarView.swift— Glass capsule with tab icons (Home, Tasks, More)ExpandedMenuView.swift— Menu list + org switcher + settings gearGlassCircleButton.swift— Reusable glass circle button (search, settings)TabBarProps.swift— Props + EventDispatchers for JS bridgeJS layer:
TabBarView.tsx—requireNativeView("TabBar")wrapped in<Host>from@expo/ui/swift-uiAuthenticatedTabBar.tsx— Bridges native view touseTabTriggerfromexpo-router/uionExpandedChangeevent)Test plan
npx expo prebuild --clean && npx expo run:iosfromapps/mobile/Summary by CodeRabbit
New Features
Refactor
Chores