-
-
Notifications
You must be signed in to change notification settings - Fork 278
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(suite-native): update social network links #17086
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request includes modifications across multiple areas of the project. In the icons generation process, two new social media icons—Instagram and TikTok—have been added, and the corresponding JSON file for mobile icon fonts has been updated by removing several icons, inserting the new ones, and adjusting the numeric mapping. The runtime configuration has been updated by changing the runtime version from '26' to '27'. Additionally, enhancements have been made to the localization support by introducing new translation keys in the internationalization file for module settings. The AboutUsBanners component saw a replacement of static text with translation components and an update of the social media icons and links, reflecting changes in the supported platforms and adjustments to accessibility labels. ✨ Finishing Touches
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
CodeRabbit Configuration File (
|
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: 0
🔭 Outside diff range comments (1)
suite-common/icons/generateIconFont.ts (1)
73-104
: 💡 Verification agent🧩 Analysis chain
LGTM!
The new social media icons are correctly added to the
usedIcons
array.Run the following script to verify the icon assets exist:
🏁 Script executed:
#!/bin/bash # Description: Verify that the icon assets exist. # Test: Check if the icon assets exist for icon in "instagramLogo" "tiktokLogo"; do if [ -f "./assets/${icon}.svg" ]; then echo "✓ ${icon}.svg exists" else echo "✗ ${icon}.svg is missing" fi doneLength of output: 298
Missing Asset Files for New Social Media Icons
The changes to the
usedIcons
array now include social media icons (instagramLogo
,tiktokLogo
), but the corresponding asset files (./assets/instagramLogo.svg
and./assets/tiktokLogo.svg
) are missing. Please add these icon assets or remove them from the array if they're not intended to be used.
- Location:
suite-common/icons/generateIconFont.ts
(Lines 73-104)- Action: Ensure that
instagramLogo.svg
andtiktokLogo.svg
exist in the./assets
folder.
🧹 Nitpick comments (4)
suite-common/icons/iconFontsMobile/TrezorSuiteIcons.json (4)
22-23
: New Social Icon Addition & 'swap' Reassignment
The introduction of"tiktokLogo": 61717
clearly replaces the previous mapping for"swap"
(which now appears on the next line as 61718). Please verify that this change aligns with the design requirements for social network links and that all components referencing these icons have been updated accordingly.
24-52
: Re-indexing of Icon Mappings (Early Section)
Icons from"starFilled": 61719
through"lifebuoy": 61747
have new numeric mappings. It is important to confirm that these updated assignments are synchronized with both the mobile icon font generator and any UI components or static references that depend on them.
53-53
: Addition of 'instagramLogo' Icon
The new"instagramLogo": 61748
entry introduces support for Instagram. Ensure that the corresponding source files (such as the icon font generator configuration) reference this new icon correctly and that its accessibility labels and usage in social network links are updated as needed.
54-110
: Re-indexing of Icon Mappings (Remaining Section)
The remainder of the icons—from"info": 61749
to"arrowDown": 61805
—have been re-indexed. This extensive renumbering could affect downstream dependencies, caching systems, or any components that rely on fixed numeric identifiers. It is advisable to perform comprehensive validation to ensure that these changes do not inadvertently break asset references or font generation processes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
suite-common/icons/iconFontsMobile/TrezorSuiteIcons.ttf
is excluded by!**/*.ttf
📒 Files selected for processing (5)
suite-common/icons/generateIconFont.ts
(2 hunks)suite-common/icons/iconFontsMobile/TrezorSuiteIcons.json
(1 hunks)suite-native/app/app.config.ts
(1 hunks)suite-native/intl/src/en.ts
(1 hunks)suite-native/module-settings/src/components/AboutUsBanners.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
🔇 Additional comments (6)
suite-native/module-settings/src/components/AboutUsBanners.tsx (4)
3-3
: LGTM!The Translation component import is correctly added to support localization.
39-39
: LGTM!Replacing static text with a Translation component improves localization support.
46-46
: LGTM!Replacing static text with a Translation component improves localization support.
52-71
:✅ Verification successful
Verify the social media links.
The social media icons, links, and accessibility labels have been updated to reflect current platforms.
Run the following script to verify the social media links:
🏁 Script executed:
#!/bin/bash # Description: Verify that the social media links are valid and accessible. # Test: Check if the social media links are accessible for url in "https://x.com/trezor" "https://www.tiktok.com/@trezor.io_official" "https://www.instagram.com/trezor.io/"; do status_code=$(curl -s -o /dev/null -w "%{http_code}" "$url") echo "Testing $url - Status code: $status_code" doneLength of output: 1031
Social media links and icons verified
- The Twitter (X) and TikTok links return HTTP 200, confirming that they load successfully.
- The Instagram link returns an HTTP 302, which is expected due to redirection, and still indicates proper accessibility.
All updated social media platforms, links, and accessibility labels have been verified and are functioning as expected.
suite-native/app/app.config.ts (1)
171-171
:❓ Verification inconclusive
Verify the runtime version update.
The runtime version has been updated from '26' to '27'.
🌐 Web query:
What is the latest Expo runtime version and what are the breaking changes between version 26 and 27?
💡 Result:
The latest Expo SDK version mentioned in the search results is 52.0.0 ([4][7]), released in November 2024 with React Native 0.76 and React Native Web 0.19.13[4]. However, there are no references to a "version 27" in any of the provided search results.
Breaking Changes Analysis
No direct information exists about breaking changes between hypothetical versions "26" and "27" of Expo runtime or SDK in these search results:
- Keycloak documentation[8] mentions version "26.1.0" with no breaking changes - but this appears unrelated to Expo.
- Expo Updates library[7] shows:
- Version 0.26.0 (Oct 2024) introduced iOS crash fixes and configuration improvements
- Version 0.25.0 (April 2024) deprecated Android initialization APIs
- Expo SDK[3][4] progression shows:
- Version 52 added DOM components and fetch API support[3]
- Version 51 focused on Android bridgeless mode support[3]
Notable Recent Changes
From SDK v49 to v52:
- Added DOM Components framework[3]
- Introduced Fetch API with
file://
support[3]- Updated iOS deployment targets to 15+[2]
- Migrated Android compression to Brotli[5]
- Added automatic update rollback protections[1]
The search results contain no evidence of an Expo runtime version "27". For production use cases requiring stability, SDK v52 remains the latest validated release as of February 2025 based on these sources[3][4].
Citations:
- 1: https://docs.expo.dev/eas-update/runtime-versions/
- 2: https://github.com/expo/expo/blob/main/CHANGELOG.md
- 3: https://github.com/expo/expo/blob/main/packages/expo/CHANGELOG.md
- 4: https://docs.expo.dev/versions/latest/
- 5: https://blog.expo.dev/expo-sdk-49-c6d398cdf740
- 6: https://expo.dev/changelog
- 7: https://github.com/expo/expo/blob/main/packages/expo-updates/CHANGELOG.md
- 8: https://www.keycloak.org/docs/latest/upgrading/index.html
- 9: https://github.com/expo/eas-cli/blob/main/CHANGELOG.md
- 10: EAS Update will fail if "runtimeVersion" is manually configured with something like "3.60" expo/eas-cli#1518
Runtime Version Update Verification Required
The runtime version in
suite-native/app/app.config.ts
(line 171) was updated from'26'
to'27'
. However, our verification indicates that there is no official reference to an Expo runtime version'27'
. The latest available Expo SDK version listed in the documentation is 52.0.0, and there is no evidence supporting a version bump from'26'
to'27'
.
- Action: Please confirm whether
'27'
is intended as a custom runtime version or if it should align with an officially supported Expo SDK version. Adjust the configuration if necessary.suite-native/intl/src/en.ts (1)
726-727
: LGTM!The new translation keys are correctly added to support the localization changes in AboutUsBanners.tsx.
accessibilityLabel="facebook" | ||
onPress={() => openLink('https://www.facebook.com/trezor.io')} | ||
accessibilityLabel="X" | ||
onPress={() => openLink('https://x.com/trezor')} |
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.
Related Issue
Resolve #17058
Screenshots: