-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(native-app): support type: 'Table' in field render - do not merge #16908
Conversation
WalkthroughThe Changes
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16908 +/- ##
==========================================
- Coverage 36.44% 36.43% -0.01%
==========================================
Files 6853 6855 +2
Lines 143622 143681 +59
Branches 41000 41019 +19
==========================================
+ Hits 52347 52356 +9
- Misses 91275 91325 +50
Flags with carried forward coverage won't be shown. Click here to find out more. see 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
|
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: 1
🧹 Outside diff range and nitpick comments (1)
apps/native/app/src/screens/wallet-pass/components/field-render.tsx (1)
73-73
: Extract hardcoded styles into a constant or themeThe styling values are duplicated. Consider extracting them into a constant or theme for better maintainability.
+ const styles = { + labelContainer: { + marginTop: 24, + paddingBottom: 4, + }, + } - <View key={key} style={{ marginTop: 24, paddingBottom: 4 }}> + <View key={key} style={styles.labelContainer}>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
apps/native/app/src/screens/wallet-pass/components/field-render.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/native/app/src/screens/wallet-pass/components/field-render.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (1)
apps/native/app/src/screens/wallet-pass/components/field-render.tsx (1)
70-70
: Verify TypeScript type definitions
Ensure that the type
field in GenericLicenseDataField
includes 'Table' as a valid value.
apps/native/app/src/screens/wallet-pass/components/field-render.tsx
Outdated
Show resolved
Hide resolved
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.
Nice!
apps/native/app/src/screens/wallet-pass/components/field-render.tsx
Outdated
Show resolved
Hide resolved
#16908) * feat: support type: 'Table' in field render * fix: use theme when relevant --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Support type 'Table' in Field render.
Do not merge until after next release (10th of December) and a go from Máni that his PR is being released then.
Why
The service portal is using this for firearm licenses. To keep everything in sync the app supports that as well now. Máni from Hugsmiðjan is creating a rule on the backend that if the user-agent is version that is older than 1.4.8 then he will send the type: 'Group' for firearm licenses to be backwards compatible, otherwise he just sends type: 'Table'
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit