Fix 2424#2443
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughThis PR introduces three independent improvements: refactors the app-setup overlay to use explicit Button and Column components with documentation diagrams; enhances the kebab menu component with testIDs and accessibility attributes to restore automation coverage for the "more options" menu; and updates the iOS Swift package dependency to pin inji-openid4vp-ios-swift version 0.7.1. ChangesSetup Incomplete Overlay Refactoring
Kebab Menu Accessibility and Testability
iOS Swift Package Dependency Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
inji-wallet_Solution (1)
1-8: Verify the relationship between this subproject update and the PR objectives.This subproject commit references PR
#2440("chore: update ovp swift package to 0.7.1"), but the current PR#2443objectives focus on adding accessibility IDs to KebabPopUp.tsx. The AI summary mentions "three independent improvements," suggesting this PR combines unrelated changes.Mixing independent changes in a single PR can:
- Complicate code review by requiring expertise in multiple areas
- Make rollback more difficult if one change needs to be reverted
- Obscure git history and make bisecting issues harder
Consider splitting unrelated changes into separate PRs, with each focused on a single concern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inji-wallet_Solution` around lines 1 - 8, This PR mixes an unrelated subproject update (commit 2b37c058a9ae10216e099dce78fbda57069d41ef referencing PR `#2440` that updates the ovp Swift package) with accessibility work for KebabPopUp.tsx in PR `#2443`; separate concerns by removing or reverting the subproject commit from this branch (or move the KebabPopUp.tsx changes into a new branch/PR) so each PR contains a single focused change; reference the commit hash 2b37c058a9ae10216e099dce78fbda57069d41ef, PR `#2440`, PR `#2443` and the KebabPopUp.tsx file when creating the separate PRs or revert to ensure history and review scope remain clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@App.tsx`:
- Around line 92-107: The "Proceed" button currently only calls
setDeepLinkOverlayVisible(false) (same as the dismiss button) but should kick
off the app setup flow; update the onPress handler for the Button with
testID="proceedToSetup" to both hide the overlay and navigate to the setup entry
point used elsewhere (e.g., invoke the same setup navigation used by
WelcomeScreenController/AuthScreenController: navigate to the Welcome or
Passcode screen with the setup flag or call the controller's startSetup method),
instead of only calling setDeepLinkOverlayVisible(false); leave the dismiss
button (testID="dismissSetupError") unchanged.
In `@components/KebabPopUp.tsx`:
- Line 55: The accessibilityLabel in KebabPopUp is hardcoded; change it to use
the i18n translator (t) instead of the literal "Close More Options" so the label
is localized (e.g., accessibilityLabel={t('kebab.closeMoreOptions')}) and add
the matching key (kebab.closeMoreOptions) with translations to your i18n
resource files; update any imports to ensure t is in scope in the KebabPopUp
component if not already.
---
Nitpick comments:
In `@inji-wallet_Solution`:
- Around line 1-8: This PR mixes an unrelated subproject update (commit
2b37c058a9ae10216e099dce78fbda57069d41ef referencing PR `#2440` that updates the
ovp Swift package) with accessibility work for KebabPopUp.tsx in PR `#2443`;
separate concerns by removing or reverting the subproject commit from this
branch (or move the KebabPopUp.tsx changes into a new branch/PR) so each PR
contains a single focused change; reference the commit hash
2b37c058a9ae10216e099dce78fbda57069d41ef, PR `#2440`, PR `#2443` and the
KebabPopUp.tsx file when creating the separate PRs or revert to ensure history
and review scope remain clean.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea6f1769-5508-451a-839f-9b7f5e95452e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
App.tsxLogic_Flowchart.excalidrawUX_Diagram.excalidrawcomponents/KebabPopUp.tsxinji-wallet_Solution
| <Button | ||
| testID="proceedToSetup" | ||
| type="gradient" | ||
| title={t('common:proceed')} | ||
| onPress={() => { | ||
| setDeepLinkOverlayVisible(false); | ||
| }} | ||
| styles={Theme.MessageOverlayStyles.button} | ||
| /> | ||
| <Button | ||
| testID="dismissSetupError" | ||
| type="clear" | ||
| title={t('common:ok')} | ||
| onPress={() => { | ||
| setDeepLinkOverlayVisible(false); | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: find the existing navigation/setup action to wire into proceedToSetup.
# Expected: one or more controller/appService actions that navigate to setup/onboarding.
rg -n -C3 --type=ts --type=tsx '\b(useApp|controller)\b'
rg -n -C3 --type=ts --type=tsx '\b(proceedToSetup|dismissSetupError|appSetupIncomplete|setup)\b'
rg -n -C3 --type=ts --type=tsx '\b(APP_EVENTS|send\().*(SETUP|ONBOARD|NAVIGAT|WALLET)\b'Repository: inji/inji-wallet
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Find App.tsx and examine the button handlers
fd -e tsx -e ts App.tsxRepository: inji/inji-wallet
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Search for setup/navigation actions without type restrictions
# Use -t js as fallback for TypeScript files
rg -n -C3 '(useApp|controller|appService)' --max-count=20
rg -n -C3 'proceedToSetup|dismissSetupError|appSetupIncomplete' --max-count=20
rg -n 'setup|onboarding|SETUP|ONBOARD' -i --max-count=15Repository: inji/inji-wallet
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Find and read App.tsx to verify the code snippet
wc -l App.tsxRepository: inji/inji-wallet
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Read lines 80-120 of App.tsx to see full context
sed -n '80,120p' App.tsxRepository: inji/inji-wallet
Length of output: 1269
🏁 Script executed:
#!/bin/bash
# Search for setup/onboarding/deep link related navigation or actions
rg -n 'setup|onboard|deepLink|isDeepLink' -i -A 2 | head -100Repository: inji/inji-wallet
Length of output: 7509
🏁 Script executed:
#!/bin/bash
# Search for setup/onboarding navigation or flow patterns
rg -n 'navigate.*setup|navigate.*onboard|goToSetup|proceedToSetup' -i
rg -n 'isAppSetupComplete' -i -B 2 -A 2Repository: inji/inji-wallet
Length of output: 1945
🏁 Script executed:
#!/bin/bash
# Search for welcome/auth screens that might handle setup
rg -n 'Welcome|Auth|Setup.*Screen|Onboard' -i | grep -i 'navigate\|route\|screen' | head -30Repository: inji/inji-wallet
Length of output: 2516
🏁 Script executed:
#!/bin/bash
# Read top 70 lines of App.tsx to see imports and function signature
head -70 App.tsxRepository: inji/inji-wallet
Length of output: 2784
Proceed to Setup action should navigate to the setup flow, not just dismiss the overlay.
Both button handlers (lines 96 and 105) only call setDeepLinkOverlayVisible(false), making proceedToSetup functionally identical to dismissSetupError. The "Proceed" button should navigate to the setup flow (e.g., the Welcome or Passcode screen with setup flag), consistent with how setup is triggered elsewhere in the app (e.g., WelcomeScreenController, AuthScreenController). This breaks the intended user path when a deep link is opened before app setup is complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@App.tsx` around lines 92 - 107, The "Proceed" button currently only calls
setDeepLinkOverlayVisible(false) (same as the dismiss button) but should kick
off the app setup flow; update the onPress handler for the Button with
testID="proceedToSetup" to both hide the overlay and navigate to the setup entry
point used elsewhere (e.g., invoke the same setup navigation used by
WelcomeScreenController/AuthScreenController: navigate to the Welcome or
Passcode screen with the setup flag or call the controller's startSetup method),
instead of only calling setDeepLinkOverlayVisible(false); leave the dismiss
button (testID="dismissSetupError") unchanged.
| {...testIDProps('close')} | ||
| {...testIDProps('closeMoreOptions')} | ||
| accessible={true} | ||
| accessibilityLabel={'Close More Options'} |
There was a problem hiding this comment.
Use i18n for the accessibilityLabel.
The accessibilityLabel is hardcoded in English. For consistency with the rest of the component (which uses the t() translation function), this should be localized.
🌐 Proposed fix to use i18n
- accessibilityLabel={'Close More Options'}
+ accessibilityLabel={t('closeButton')}Make sure to add the corresponding key to the translation files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/KebabPopUp.tsx` at line 55, The accessibilityLabel in KebabPopUp
is hardcoded; change it to use the i18n translator (t) instead of the literal
"Close More Options" so the label is localized (e.g.,
accessibilityLabel={t('kebab.closeMoreOptions')}) and add the matching key
(kebab.closeMoreOptions) with translations to your i18n resource files; update
any imports to ensure t is in scope in the KebabPopUp component if not already.
Signed-off-by: Rucha <imt_2025071@iiitm.ac.in>
) - Added testID and accessibilityLabel to each ListItem in the kebab menu - Added descriptive testID to the Overlay container (moreOptionsOverlay) - Updated close button with accessibilityLabel and accessibilityRole - Ensures screen readers (VoiceOver/TalkBack) can navigate all menu options - Enables automated E2E tests to target individual menu items Signed-off-by: Rucha <imt_2025071@iiitm.ac.in>
1 participant
Summary
Fixes #2424
The "More Options" (kebab) popup menu was missing accessibility IDs on its interactive
elements, making it impossible for screen readers (VoiceOver/TalkBack) and automated E2E
test frameworks (Detox/Appium) to interact with individual menu items.
Changes Made
File:
components/KebabPopUp.tsxtestID="moreOptionsOverlay"testID="close"(generic)testID="closeMoreOptions"+accessibilityLabel+accessibilityRoletestID="kebabMenu-{itemTestID}"+accessibilityLabel+accessibilityRoleAccessibility IDs Added
moreOptionsOverlay— The overlay containercloseMoreOptions— The close (X) buttonkebabMenu-pinOrUnPinCard— Pin/Unpin card optionkebabMenu-viewActivityLog— View activity log optionkebabMenu-shareVcFromKebab— Share VC optionkebabMenu-shareVcWithSelfieFromKebab— Share with selfie optionkebabMenu-reverify— Reverify optionkebabMenu-removeFromWallet— Remove from wallet optionkebabMenu-pendingActivationOrActivated— Activation optionTesting
testIDProps()pattern fromshared/commonUtil.tsSummary by CodeRabbit
New Features
Accessibility
Chores