-
Notifications
You must be signed in to change notification settings - Fork 180
bugfix: update migrated components default text color #1271
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
Conversation
WalkthroughDocumentation expanded with installation, linking, migration, initialization, styling, and SDK events guidance. Typography components updated to apply new colors and use flattened styles. A web-only button now perserves animation state via a ref to Animated.Value. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant SDK
participant ScannerAdapter
participant NetworkAdapter
participant CryptoAdapter
participant Storage(No-op)
participant Logger(No-op)
App->>SDK: initialize({ scanner, network, crypto, storage?, clock?, logger? })
activate SDK
SDK->>ScannerAdapter: configure
SDK->>NetworkAdapter: configure
SDK->>CryptoAdapter: configure
Note over SDK,Storage: Defaults to no-op storage/clock/logger if omitted
SDK-->>App: ready()
deactivate SDK
sequenceDiagram
autonumber
participant AppUI
participant SDK
participant EventBus
AppUI->>SDK: user selects document
SDK->>EventBus: emit(DocumentSelected, payload)
AppUI-->>AppUI: update UI
AppUI->>SDK: start verification flow
SDK->>EventBus: emit(VerificationStarted)
SDK->>EventBus: emit(VerificationProgress|Error|Completed)
AppUI-->>AppUI: respond to events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)packages/mobile-sdk-alpha/**/*.{ts,tsx}📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
Files:
**/*.{js,ts,tsx,jsx,sol,nr}📄 CodeRabbit inference engine (.cursorrules)
Files:
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (2)packages/mobile-sdk-alpha/src/components/typography/Caption.tsx (3)
packages/mobile-sdk-alpha/src/components/typography/Title.tsx (2)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
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 |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/mobile-sdk-alpha/README.md(3 hunks)packages/mobile-sdk-alpha/src/components/buttons/PrimaryButtonLongHold.web.tsx(2 hunks)packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx(1 hunks)packages/mobile-sdk-alpha/src/components/typography/Caption.tsx(2 hunks)packages/mobile-sdk-alpha/src/components/typography/Title.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/components/buttons/PrimaryButtonLongHold.web.tsxpackages/mobile-sdk-alpha/src/components/typography/Caption.tsxpackages/mobile-sdk-alpha/src/components/typography/Title.tsxpackages/mobile-sdk-alpha/src/components/typography/BodyText.tsx
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/components/buttons/PrimaryButtonLongHold.web.tsxpackages/mobile-sdk-alpha/src/components/typography/Caption.tsxpackages/mobile-sdk-alpha/src/components/typography/Title.tsxpackages/mobile-sdk-alpha/src/components/typography/BodyText.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/components/buttons/PrimaryButtonLongHold.web.tsxpackages/mobile-sdk-alpha/src/components/typography/Caption.tsxpackages/mobile-sdk-alpha/src/components/typography/Title.tsxpackages/mobile-sdk-alpha/src/components/typography/BodyText.tsx
packages/mobile-sdk-alpha/README.md
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Files:
packages/mobile-sdk-alpha/README.md
🧠 Learnings (1)
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/README.md
🧬 Code graph analysis (3)
packages/mobile-sdk-alpha/src/components/typography/Caption.tsx (3)
packages/mobile-sdk-demo/tests/mocks/react-native.ts (1)
StyleSheet(27-29)packages/mobile-sdk-alpha/src/constants/colors.ts (1)
slate400(39-39)packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx (1)
BodyText(12-14)
packages/mobile-sdk-alpha/src/components/typography/Title.tsx (2)
packages/mobile-sdk-alpha/src/constants/colors.ts (1)
black(8-8)packages/mobile-sdk-demo/tests/mocks/react-native.ts (2)
StyleSheet(27-29)Text(31-31)
packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx (2)
packages/mobile-sdk-alpha/src/utils/fonts.ts (1)
dinot(8-8)packages/mobile-sdk-alpha/src/constants/colors.ts (1)
slate500(43-43)
🪛 markdownlint-cli2 (0.18.1)
packages/mobile-sdk-alpha/README.md
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
143-143: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
149-149: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build-deps
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: android-e2e
- GitHub Check: ios-e2e
🔇 Additional comments (7)
packages/mobile-sdk-alpha/src/components/buttons/PrimaryButtonLongHold.web.tsx (1)
5-5: LGTM! Proper fix for Animated.Value persistence.Using
useRefto persist theAnimated.Valueacross renders is the correct pattern. This prevents the value from being recreated on every render, which would break the animation state and cause performance issues.The implementation is clean:
- The ref is initialized once with
new Animated.Value(0)- The current value is extracted for convenient access
- It's used consistently throughout the component
Also applies to: 17-18
packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx (2)
9-9: LGTM! Default color addition aligns with bugfix objective.The addition of
slate500as the default text color is appropriate for this base typography component. The style merging order correctly allows user-provided styles to override the default color.Note: BodyText retains inline array style merging while Title and Caption use
StyleSheet.flatten. This difference is acceptable since BodyText is a primitive component with minimal style logic.Also applies to: 13-13
9-9: LGTM! Default color applied correctly.The addition of the
slate500color constant provides a consistent default text color for the BodyText component, aligning with the PR objective to update migrated component colors.Also applies to: 13-13
packages/mobile-sdk-alpha/src/components/typography/Title.tsx (3)
22-22: Default color addition looks good.Adding
blackas the default text color for Title components is appropriate and aligns with the PR's bugfix objective.
7-7: LGTM! Default color and style imports added correctly.The addition of
StyleSheetandblackcolor imports, along with the explicitcolor: blackin the base style, ensures consistent default styling for Title components.Also applies to: 9-9, 22-22
33-34: LGTM! Style flattening applied correctly.The use of
StyleSheet.flattenconsolidates the style array into a single style object, which aligns with the PR's approach to centralized style merging across typography components.Also applies to: 36-36
packages/mobile-sdk-alpha/src/components/typography/Caption.tsx (1)
6-6: LGTM! Style flattening and color override implemented correctly.The use of
StyleSheet.flattenwithslate400color ensures Caption properly overrides the defaultslate500color from the underlying BodyText component, providing the correct visual hierarchy for caption text.Also applies to: 17-17, 19-19
Summary by CodeRabbit
Documentation
Style
Refactor