Skip to content

feat: add license and telemetry settings#49

Draft
bcye wants to merge 1 commit intomainfrom
codex/add-nested-stack-to-settings-tab
Draft

feat: add license and telemetry settings#49
bcye wants to merge 1 commit intomainfrom
codex/add-nested-stack-to-settings-tab

Conversation

@bcye
Copy link
Owner

@bcye bcye commented Sep 9, 2025

Summary

  • nest settings tab under its own stack
  • add licenses viewer sourcing pnpm licenses list
  • allow toggling Sentry telemetry with in-app switch

Testing

  • pnpm test
  • pnpm lint
  • pnpm type-check

https://chatgpt.com/codex/tasks/task_e_68c08dbe13e88333bb17f09fc44693b2

Summary by CodeRabbit

  • New Features

    • Added a functional Settings screen with a telemetry (Sentry) toggle that persists your choice and applies it after app reload.
    • Introduced a Licenses section with a list of third‑party packages and detailed views for each license.
  • Enhancements

    • Settings now supports nested navigation.
    • Settings header is hidden to match global header behavior.
    • Improved cross‑platform scrolling consistency.
  • Removed

    • Removed the unused placeholder Settings screen.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Introduces a nested Settings stack with a new Settings screen, licenses list/detail routes, and a cross-platform ScrollView UI component. Removes the previous placeholder Settings screen. Adjusts main layout to hide the Settings header. Adds AsyncStorage-backed Sentry consent toggle and DevSettings-based app reload.

Changes

Cohort / File(s) Summary of changes
Main layout adjustment
app/app/main/_layout.tsx
Updated Settings tab options to hide header (headerShown: false).
Removed placeholder screen
app/app/main/settings.tsx
Deleted no-op Settings screen component.
Settings stack layout
app/app/main/settings/_layout.tsx
Added minimal Stack layout for nested Settings routes.
Settings screen
app/app/main/settings/index.tsx
Added Settings UI with Licenses navigation and Sentry telemetry toggle using AsyncStorage and DevSettings.reload(). Exports options: { title: "Settings" }.
Licenses list
app/app/main/settings/licenses/index.tsx
Added licenses list screen loading from licenses.json, navigates to [pkg]. Exports options: { title: "Licenses" }.
License detail
app/app/main/settings/licenses/[pkg].tsx
Added license detail screen using route param; shows license text or fallback. Exports dynamic options setting title to pkg.
UI ScrollView (native)
app/components/ui/scroll-view/index.tsx
Introduced RN ScrollView wrapper with variant styling via scrollViewStyle; forwardRef and displayName set.
UI ScrollView (web)
app/components/ui/scroll-view/index.web.tsx
Added web ScrollView wrapper rendering a div with variant styling; forwardRef and displayName set.
UI ScrollView styles
app/components/ui/scroll-view/styles.tsx
Added scrollViewStyle via TVA with web-aware base styles.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Settings as Settings Screen
  participant Storage as AsyncStorage
  participant Dev as DevSettings
  participant Router as Expo Router

  User->>Settings: Open Settings
  activate Settings
  Settings->>Storage: getItem("sentryConsent")
  Storage-->>Settings: "1" | "0" | null
  Settings-->>User: Render toggle (on/off)
  User->>Settings: Tap "Licenses"
  Settings->>Router: navigate("/main/settings/licenses")
  deactivate Settings
Loading
sequenceDiagram
  autonumber
  actor User
  participant List as Licenses List
  participant Detail as License Detail
  participant Data as licenses.json
  participant Router as Expo Router

  User->>List: Open Licenses
  List->>Data: import licenses
  Data-->>List: licenses[]
  User->>List: Tap package row (pkg)
  List->>Router: navigate("/.../licenses/[pkg]", { pkg })
  Router-->>Detail: params.pkg
  Detail->>Data: find license by pkg
  Data-->>Detail: license | undefined
  Detail-->>User: Render text or "License not found"
Loading
sequenceDiagram
  autonumber
  actor User
  participant Settings as Settings Screen
  participant Storage as AsyncStorage
  participant Dev as DevSettings

  User->>Settings: Toggle Sentry
  Settings->>Storage: setItem("sentryConsent", "1"/"0")
  Storage-->>Settings: Promise resolved
  Settings->>Dev: reload()
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Title Check ✅ Passed The pull request title “feat: add license and telemetry settings” is concise and focuses on the primary user-facing functionality—adding the license viewer and in-app telemetry toggle—aligning with the main objectives of the changeset even though it omits the stack nesting implementation detail, making it sufficiently descriptive for a quick history scan.
Description Check ✅ Passed The description outlines each of the key changes—nesting the settings tab under its own stack, integrating the license viewer sourced from pnpm licenses list, and adding the Sentry telemetry toggle—directly reflecting the modifications in the pull request and including relevant testing commands, so it is on-topic and appropriately informative for this context.

Poem

I twitch my ears at toggles bright,
A hop to Licenses, left then right—
Scrolls unfurl like clover leaves,
Consent saved snug in storage eaves.
With a thump, reload! fresh and new,
Settings stack where carrots grew.
Bun-approved, we ship this through.

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.

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.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-nested-stack-to-settings-tab

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.

@bcye bcye marked this pull request as draft September 9, 2025 20:45
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: 2

🧹 Nitpick comments (6)
app/app/main/_layout.tsx (1)

60-66: Redundant headerShown override on Settings tab

You already set headerShown: false in the parent screenOptions. This per-screen override is unnecessary.

           options={{
             title: "Settings",
             tabBarIcon: getTabBarIcon("Settings", "cog"),
-            headerShown: false,
           }}
app/app/main/settings/licenses/index.tsx (2)

5-5: Verify import path for licenses.json (alias vs relative path)

import licenses from "@/licenses.json"; assumes @ resolves to the directory containing licenses.json. If the file actually lives next to this module (e.g., app/app/main/settings/licenses/licenses.json), the alias will fail on native/web builds. Prefer a relative import to make it location-agnostic.

- import licenses from "@/licenses.json";
+ import licenses from "./licenses.json";

10-26: Minor UX: make rows accessible and deterministic order

  • Add accessibility roles/states for better a11y.
  • Consider sorting by package name for stable list order.
-    <ScrollView className="flex-1 p-4">
-      {licenses.map((pkg) => (
-        <Pressable
+    <ScrollView className="flex-1 p-4">
+      {licenses
+        .slice()
+        .sort((a, b) => a.name.localeCompare(b.name))
+        .map((pkg) => (
+        <Pressable
           key={pkg.name}
           className="py-2 border-b border-gray-200"
-          onPress={() =>
+          accessibilityRole="button"
+          onPress={() =>
             router.push({
               pathname: "/main/settings/licenses/[pkg]",
               params: { pkg: pkg.name },
             })
           }
         >
app/app/main/settings/licenses/[pkg].tsx (2)

4-4: Verify licenses.json import path here as well

Match the approach used in the list screen to avoid alias resolution issues.

- import licenses from "@/licenses.json";
+ import licenses from "./licenses.json";

14-16: Preserve formatting across platforms and improve readability

whitespace-pre-wrap only affects web. For RN, newlines are respected but multiple spaces may collapse. Consider making the text selectable and using a monospace font to improve license readability.

-      <Text className="whitespace-pre-wrap">
+      <Text selectable className="whitespace-pre-wrap font-mono">
         {info?.text || "License not found"}
       </Text>
app/app/main/settings/index.tsx (1)

35-50: A11y: mark the custom switch as a real switch

Expose correct accessibility metadata so screen readers announce state changes.

-        <Pressable onPress={toggleSentry}>
+        <Pressable
+          onPress={toggleSentry}
+          accessibilityRole="switch"
+          accessibilityState={{ checked: sentryEnabled }}
+          aria-checked={sentryEnabled}
+        >
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfe5c16 and 1865960.

📒 Files selected for processing (9)
  • app/app/main/_layout.tsx (1 hunks)
  • app/app/main/settings.tsx (0 hunks)
  • app/app/main/settings/_layout.tsx (1 hunks)
  • app/app/main/settings/index.tsx (1 hunks)
  • app/app/main/settings/licenses/[pkg].tsx (1 hunks)
  • app/app/main/settings/licenses/index.tsx (1 hunks)
  • app/components/ui/scroll-view/index.tsx (1 hunks)
  • app/components/ui/scroll-view/index.web.tsx (1 hunks)
  • app/components/ui/scroll-view/styles.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • app/app/main/settings.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
app/components/ui/scroll-view/styles.tsx (4)
app/components/ui/gluestack-ui-provider/index.web.tsx (3)
  • window (68-81)
  • GluestackUIProvider (22-96)
  • mode (48-57)
app/components/ui/hstack/index.web.tsx (1)
  • HStack (9-17)
app/components/ui/box/index.tsx (1)
  • Box (11-15)
app/components/ui/center/index.web.tsx (1)
  • Center (9-16)
app/app/main/settings/index.tsx (3)
app/app/main/settings/licenses/[pkg].tsx (1)
  • options (21-23)
app/app/main/settings/licenses/index.tsx (1)
  • options (30-32)
app/app/_layout.tsx (5)
  • Layout (15-124)
  • setPrivacyConsent (105-110)
  • setPrivacyConsent (94-99)
  • AsyncStorage (21-26)
  • sentryConsent (28-63)
app/app/main/settings/licenses/[pkg].tsx (4)
app/components/ui/scroll-view/index.tsx (1)
  • ScrollView (23-23)
app/components/ui/scroll-view/index.web.tsx (1)
  • ScrollView (15-15)
app/app/main/settings/index.tsx (1)
  • options (55-57)
app/app/main/settings/licenses/index.tsx (1)
  • options (30-32)
app/app/main/settings/_layout.tsx (2)
app/app/(tabs)/settings.tsx (1)
  • Settings (3-5)
app/app/(tabs)/_layout.tsx (1)
  • TabLayout (35-72)
app/components/ui/scroll-view/index.web.tsx (2)
app/components/ui/scroll-view/styles.tsx (1)
  • scrollViewStyle (8-10)
app/hooks/use-scroll-ref.tsx (1)
  • ScrollRefProvider (18-23)
app/app/main/settings/licenses/index.tsx (5)
app/components/ui/scroll-view/index.tsx (1)
  • ScrollView (23-23)
app/components/ui/scroll-view/index.web.tsx (1)
  • ScrollView (15-15)
app/components/ui/pressable/index.tsx (1)
  • Pressable (39-39)
app/app/main/settings/index.tsx (1)
  • options (55-57)
app/app/main/settings/licenses/[pkg].tsx (1)
  • options (21-23)
app/app/main/_layout.tsx (1)
app/app/(tabs)/settings.tsx (1)
  • Settings (3-5)
app/components/ui/scroll-view/index.tsx (3)
app/components/ui/scroll-view/styles.tsx (1)
  • scrollViewStyle (8-10)
app/hooks/use-scroll-ref.tsx (2)
  • ScrollRefProvider (18-23)
  • useContext (25-25)
app/components/render-node.tsx (1)
  • scrollRef (239-242)
🪛 GitHub Actions: Lint App
app/components/ui/scroll-view/index.web.tsx

[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.

🔇 Additional comments (3)
app/app/main/settings/_layout.tsx (1)

1-5: LGTM: minimal nested Stack layout

Simple and correct for hosting the nested Settings routes.

app/components/ui/scroll-view/styles.tsx (1)

1-10: LGTM: web-only base class application

Using isWeb to scope the base class set to web avoids RN style warnings, and centralizing via tva is consistent with other UI wrappers.

app/components/ui/scroll-view/index.tsx (1)

1-24: LGTM: RN ScrollView wrapper with TVA-based className

Typing, ref-forwarding, and class composition look good.

Comment on lines +20 to +25
const toggleSentry = async () => {
const next = !sentryEnabled;
setSentryEnabled(next);
await AsyncStorage.setItem("sentryConsent", next ? "1" : "0");
DevSettings.reload();
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Production-safe reload: prefer expo-updates over DevSettings.reload()

DevSettings.reload() is dev-only and won’t work in production. Use expo-updates when available, falling back to DevSettings.reload().

-import { DevSettings } from "react-native";
+import { DevSettings } from "react-native";
+import * as Updates from "expo-updates";
@@
   const toggleSentry = async () => {
     const next = !sentryEnabled;
     setSentryEnabled(next);
     await AsyncStorage.setItem("sentryConsent", next ? "1" : "0");
-    DevSettings.reload();
+    if (Updates?.reloadAsync) {
+      await Updates.reloadAsync();
+    } else if (DevSettings?.reload) {
+      DevSettings.reload();
+    }
   };
📝 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.

Suggested change
const toggleSentry = async () => {
const next = !sentryEnabled;
setSentryEnabled(next);
await AsyncStorage.setItem("sentryConsent", next ? "1" : "0");
DevSettings.reload();
};
// at the top of app/app/main/settings/index.tsx
import { DevSettings } from "react-native";
import * as Updates from "expo-updates";
// …
const toggleSentry = async () => {
const next = !sentryEnabled;
setSentryEnabled(next);
await AsyncStorage.setItem("sentryConsent", next ? "1" : "0");
if (Updates?.reloadAsync) {
await Updates.reloadAsync();
} else if (DevSettings?.reload) {
DevSettings.reload();
}
};
🤖 Prompt for AI Agents
In app/app/main/settings/index.tsx around lines 20 to 25, the toggleSentry
function calls DevSettings.reload() which only works in dev builds; replace that
call with expo-updates' reload when available and fall back to
DevSettings.reload() for older setups. Import { Updates } from 'expo-updates'
(or require dynamically), then after persisting the consent call await
Updates.reloadAsync() when Updates.isAvailable is true (or wrap in try/catch),
otherwise call DevSettings.reload(); ensure the reload invocation is awaited or
handled and gracefully falls back on error.


const ScrollView = React.forwardRef<React.ComponentRef<"div">, IScrollViewProps>(
function ScrollView({ className, ...props }, ref) {
return <div ref={ref} {...props} className={scrollViewStyle({ class: className })} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Prettier failure by wrapping long JSX line

CI flagged a Prettier issue. Wrap props across lines.

-    return <div ref={ref} {...props} className={scrollViewStyle({ class: className })} />;
+    return (
+      <div
+        ref={ref}
+        {...props}
+        className={scrollViewStyle({ class: className })}
+      />
+    );
📝 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.

Suggested change
return <div ref={ref} {...props} className={scrollViewStyle({ class: className })} />;
return (
<div
ref={ref}
{...props}
className={scrollViewStyle({ class: className })}
/>
);
🤖 Prompt for AI Agents
In app/components/ui/scroll-view/index.web.tsx around line 10, the single long
JSX return line causes Prettier failure; split the JSX attributes across
multiple lines so the element is wrapped: place the opening tag on its own line,
put ref={ref}, {...props}, and className={scrollViewStyle({ class: className })}
each on separate indented lines, and close the tag on its own line to satisfy
line-length/formatting rules.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant