chore(example): match UI with native android example

[sherwinski]

Description

One Line Summary

Updates the React Native example app to more closely follow the appearance and behavior of the Android-SDK.

Details

Motivation

To standardize testing, we want a consistent example app that ensures that this SDK exposes and wraps the correct OneSignal APIs.

Scope

Only affects the example app included in this repository

Testing

Unit testing

OPTIONAL - Explain unit tests added, if not clear in the code.

Manual testing

RECOMMEND - OPTIONAL - Explain what scenarios were tested and the environment.
Example: Tested opening a notification while the app was foregrounded, app build with Android Studio 2020.3 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
  • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
  • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
• I have reviewed this PR myself, ensuring it meets each checklist item
  • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

---

This change is 

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[sherwinski]

@claude review this PR for bare-bones React Native app best practices

[claude]

Claude finished @sherwinski's task in 1m 50s —— View job

---

React Native Best Practices Review

• Read repository guidelines (CLAUDE.md)
• Review package.json and dependencies
• Review component architecture and organization
• Check for React Native best practices compliance
• Review code quality and patterns
• Post comprehensive feedback

---

Summary

This PR significantly expands the example app from a simple demo to a comprehensive SDK showcase. While the implementation is solid overall, there are several concerns about whether this qualifies as "bare-bones" given the scope of changes (10,605 additions across 50 files).

✅ Strengths

1. Good TypeScript Usage

• Proper type definitions in types/index.ts
• Type-safe component props throughout
• No use of any types (uses unknown appropriately)

2. Well-Structured Components

• Good separation of concerns (common, dialogs, sections)
• Reusable components (Card, ActionButton, SectionHeader, etc.)
• Consistent naming conventions

3. State Management

• Context API with reducer pattern is appropriate for this scale
• Clean implementation in context/AppStateContext.tsx:1
• Type-safe actions and state

4. Security Awareness

• Good warning comments about API keys in client code (constants/Config.ts:5-7, services/NotificationSender.ts:7-15)
• Uses .env.example for sensitive config

5. Event Cleanup

• Proper event listener cleanup in OSDemo.tsx:204-241
• Uses useFocusEffect correctly for navigation-aware listeners

⚠️ Concerns for a "Bare-Bones" Example

1. Scope Creep - Not "Bare-Bones"

This doesn't feel like a bare-bones example anymore:

• 39 new files created (components, services, constants)
• 10,605 lines added vs 44 deleted
• 234-line notification sender service with comprehensive error handling
• 292-line notification payload definitions with Firebase Storage URLs
• Multiple dialog types, 15 section components, context state management

Recommendation: Consider whether this is meant to be:

• A minimal example showing basic SDK integration → current implementation is too complex
• A comprehensive demo showcasing all features → current implementation is good, but rename to reflect this

2. Dependencies - Minimal Additions ✓

Good restraint shown:

"@react-navigation/bottom-tabs": "^7.8.12",
"@react-navigation/native": "^7.1.25", 
"react-native-safe-area-context": "^5.6.2",
"react-native-screens": "^4.18.0",
"react-native-dotenv": "^3.4.11"

Only navigation and dotenv added - appropriate for the scope.

3. Hard-Coded App ID

The App ID is hard-coded in multiple places:

• constants/Config.ts:3
• DetailsScreen.tsx:111-113 (hard-coded string instead of importing from Config)

Issue: DetailsScreen.tsx:111-113 should import APP_ID from Config instead of duplicating the string.

4. Over-Engineering for a Demo

services/NotificationSender.ts (234 lines)

• Comprehensive error handling (good for production, overkill for demo)
• Platform-specific payload building
• Multiple validation layers
• Could be simplified to ~50 lines for a bare-bones example

constants/NotificationPayloads.ts (292 lines)

• 8 notification types × 3 variations = 24 templates
• Firebase Storage URLs with token mapping
• This feels like feature showcase rather than minimal example

5. Performance Concerns

FlatList Usage Without Optimization
Example in components/sections/TagsSection.tsx:49-66:

<FlatList
  data={state.tags}
  keyExtractor={(item) => item.key}
  renderItem={({ item }) => (
    <View style={styles.item}>
      {/* render logic */}
    </View>
  )}
/>

Missing:

• No scrollEnabled={false} for nested FlatLists in ScrollView
• No removeClippedSubviews optimization
• renderItem not memoized (creates new function on every render)

Impact: With 15 section components rendering FlatLists inside a parent ScrollView, this could cause performance issues.

6. Modal Accessibility

components/dialogs/BaseDialog.tsx:34-39

<Modal
  visible={visible}
  transparent
  animationType="fade"
  onRequestClose={onClose}
>

Missing: No accessibilityViewIsModal prop for iOS accessibility

7. useEffect Dependencies

components/sections/PrivacyConsentSection.tsx:18-30

useEffect(() => {
  const checkConsent = async () => {
    if (!state.consentGiven && !showModal) {
      setTimeout(() => {
        setShowModal(true);
      }, 500);
    }
  };
  checkConsent();
}, []); // ⚠️ Missing dependencies: state.consentGiven, showModal

Issue: Dependencies array is empty but references state.consentGiven and showModal. This will only run once on mount, which may be intentional, but should be commented or use proper dependencies with a ref to prevent re-triggers.

8. Inline Styles in BaseDialog

components/dialogs/BaseDialog.tsx:116

gap: 8,

Issue: gap property is not supported in React Native StyleSheet on older versions. Should use marginLeft/marginRight instead for broader compatibility.

9. Context Re-renders

context/AppStateContext.tsx:108-115

return (
  <AppStateContext.Provider value={{ state, dispatch }}>
    {children}
  </AppStateContext.Provider>
);

Issue: The value object { state, dispatch } is recreated on every render, causing unnecessary re-renders of all consumers. Should use useMemo:

const value = useMemo(() => ({ state, dispatch }), [state, dispatch]);
return (
  <AppStateContext.Provider value={value}>
    {children}
  </AppStateContext.Provider>
);

10. Inconsistent Error Handling

DetailsScreen.tsx:29-45

const loadUserInfo = async () => {
  try {
    const osId = await OneSignal.User.getOnesignalId();
    setOnesignalId(osId);
    // ... more async calls
  } catch (error) {
    console.error('Error loading user info:', error);
    // No user feedback - error silently logged
  }
};

Issue: Errors are logged but not shown to the user. At minimum, set error state to show "Failed to load" message.

11. Magic Numbers

Throughout the codebase:

• OSDemo.tsx:321-324: flex: 0.3 and flex: 0.7 (30/70 split)
• PrivacyConsentSection.tsx:24: setTimeout(..., 500)
• OSDemo.tsx:68: setTimeout(..., 5000)

Recommendation: Extract to named constants for clarity:

const CONSOLE_HEIGHT_RATIO = 0.3;
const CONTENT_HEIGHT_RATIO = 0.7;
const CONSENT_MODAL_DELAY = 500;
const NOTIFICATION_DISPLAY_DELAY = 5000;

📋 Recommendations

For "Bare-Bones" Examples:

1. Simplify or Rename: Either drastically simplify this to ~5 files, or rename from "example" to "demo" or "showcase"
2. Remove NotificationSender service: Too complex for minimal example
3. Reduce variations: One notification template instead of 24
4. Fewer sections: Show 3-4 key features instead of 15

For Current Implementation (if keeping as comprehensive demo):

1. Fix hard-coded App ID in DetailsScreen.tsx:111-113
2. Add missing useEffect dependencies or document why they're omitted
3. Memoize context provider value to prevent unnecessary re-renders
4. Replace gap with compatible spacing
5. Add user-facing error messages (not just console.error)
6. Extract magic numbers to constants
7. Add FlatList optimizations for nested lists

Verdict

For a comprehensive SDK demo: ✅ Well-implemented
For a bare-bones example: ❌ Too complex

The code quality is good, but the scope has grown significantly. This is more of a full-featured demo app than a minimal example. Consider splitting into:

• Minimal example: Basic OneSignal setup + 1-2 features (~5 files)
• Comprehensive demo: Current implementation (renamed)

---

[fadi-george]

Can you rebase

[sherwinski]

@fadi-george done

[fadi-george]

Why do we need the rest api key in Config.ts?
I don't have an .env in the demo app so its crashing when trying to run it. But I don't think we should need one.
You only need APP_ID for the demo.

[fadi-george]

Nit: Maybe we can all have the demo example named as examples/demo

[sherwinski]

Why do we need the rest api key in Config.ts? I don't have an .env in the demo app so its crashing when trying to run it. But I don't think we should need one. You only need APP_ID for the demo.

Removed