fix: disable ledger for firefox, closes #5287 #5583
Conversation
WalkthroughThe changes introduce a new constant Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
pete-watters
force-pushed
the
fix/5287/ledger-firefox
branch
from
afa2e6b to
4f77959
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/ui/pages/welcome.layout.tsx (3 hunks)
Additional comments not posted (2)
src/app/ui/pages/welcome.layout.tsx (2)
1-1: Imports are correctly added and structured.
116-116: Conditional disabling of the button is correctly implemented.Ensure to verify the UI's behavior across different browsers to ensure that the disabling functionality is consistent and does not affect user experience negatively.
Verification successful
Conditional disabling of the button is correctly implemented.
Ensure that the
isFirefoxflag is correctly identifying the Firefox browser to guarantee the button's behavior is as expected.
- Verify the implementation of
isFirefoxwithin the codebase.
Conditional disabling of the button is correctly implemented.
The
isFirefoxflag correctly identifies the Firefox browser using the user agent string.
- Implementation found in
src/app/ui/pages/welcome.layout.tsx:const isFirefox = navigator.userAgent.toLowerCase().includes('firefox');Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the UI's behavior across different browsers for consistency. # Test: Manual testing is required here. Use BrowserStack or a similar service to check the UI on different browsers. echo "Manual testing required: Check UI on different browsers using BrowserStack."Length of output: 160
Script:
#!/bin/bash # Search for the implementation of the isFirefox flag in the codebase rg 'isFirefox' --type js --type tsLength of output: 216
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
src/app/ui/pages/welcome.layout.tsx (2)
22-22: Review browser detection logic.The logic to detect Firefox using
navigator.userAgent.toLowerCase().includes('firefox')is straightforward and commonly used. However, consider the implications of user-agent sniffing and potential issues with user-agent strings in different environments.
117-117: Ensure graceful degradation for the disabled 'Use Ledger' button.The button is disabled based on the browser type. Ensure that this is clearly communicated to the user, possibly through a tooltip or an alternative message, to improve user experience and accessibility.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/app/ui/pages/welcome.layout.tsx (2 hunks)
Additional comments not posted (1)
src/app/ui/pages/welcome.layout.tsx (1)
Line range hint
4-4: Confirm usage of imported components.Ensure that
Button,LettermarkIcon,Link, andLogomarkIconare utilized appropriately in the component and that there are no unused imports.Verification successful
Confirm usage of imported components.
The imported components
Button,LettermarkIcon,Link, andLogomarkIconare all utilized in thesrc/app/ui/pages/welcome.layout.tsxfile.
Buttonis used multiple times.LettermarkIconis used.Linkis used.LogomarkIconis used.No unused imports were found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the newly imported components are utilized in the file. # Test: Search for the usage of the new components. Expect: At least one occurrence of each component. rg --type tsx $'Button|LettermarkIcon|Link|LogomarkIcon' src/app/ui/pages/welcome.layout.tsxLength of output: 121
Script:
#!/bin/bash # Description: Verify that the newly imported components are utilized in the file. # Test: Search for the usage of the new components. Expect: At least one occurrence of each component. rg 'Button|LettermarkIcon|Link|LogomarkIcon' src/app/ui/pages/welcome.layout.tsxLength of output: 744
There was a problem hiding this comment.
We have a screen for this, that shows a "browser not supported" message
IMO this is a degraded experience, because now the user doesn't know why they can't use Ledger.
...but also, we don't support FF so 🤷🏼 @314159265359879 is this still an issue? Doesn't the screen work?
Thanks, I didn't know about that screen |
This PR adds a check for
firefoxand if so disabled theUse Ledgerbutton at sign inSummary by CodeRabbit