-
Notifications
You must be signed in to change notification settings - Fork 253
Feat/mur use widget context #2352
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
base: feat/modal-ui-revamp
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
||
| if (chainNamespaces.length === 0) { | ||
| return null; | ||
| } |
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.
Chain filter shows unnecessarily with single chain namespace
Medium Severity
The chain filter component's visibility condition changed incorrectly. Previously, ConnectWalletChainFilter was only rendered when chainNamespace.length > 1. Now it's rendered unconditionally and returns null only when chainNamespaces.length === 0. This means with exactly one chain namespace, users see a redundant filter showing "All Chains" plus one chain option, which provides no useful filtering capability.
Additional Locations (1)
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.
fixed change to > 1
| */ | ||
| function BottomSheet({ isShown, onClose, children, uiConfig, sheetClassName, showCloseButton = true }: BottomSheetProps) { | ||
| const { borderRadiusType = "large" } = uiConfig; | ||
| function BottomSheet({ isShown, onClose, children, sheetClassName, showCloseButton = true, borderRadiusType = "large" }: BottomSheetProps) { |
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.
just pass borderRadius type instead of whole uiCofig
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
| const { analytics } = useContext(AnalyticsContext); | ||
| const { appLogo, deviceDetails, uiConfig } = useWidget(); | ||
| const { | ||
| buttonRadiusType: buttonRadius = "pill", |
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.
Redundant default value for buttonRadiusType
Low Severity
The default value = "pill" for buttonRadiusType is redundant. WidgetContext.tsx already sets this default in its useMemo (line 56: buttonRadiusType: "pill"), guaranteeing the value will never be undefined when accessed via useWidget(). Having the same default in two places creates a maintenance risk where future changes to the default might only update one location.
| os, | ||
| platform, | ||
| SocialLoginEventType, | ||
| SocialLoginsConfig, |
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.
Unused type properties in LoginProps interface
Low Severity
The appLogo and appName properties in LoginProps are no longer used. The Login component now obtains appLogo from useWidget() instead of props, and appName has never been used (only referenced in a TODO comment). These properties are not passed from Root.tsx to Login, making them dead interface members that could confuse future developers.
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
| os, | ||
| platform, | ||
| SocialLoginEventType, | ||
| SocialLoginsConfig, |
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.
Unused type properties in LoginProps interface
Low Severity
LoginProps interface defines appLogo and appName properties that are never passed to the Login component anymore. After refactoring, appLogo is obtained from useWidget() inside the component, and appName is not used at all. These type properties are now dead code and can be removed from the interface.


Motivation and Context
Jira Link:
Description
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Note
Medium Risk
Moderate UI refactor that removes several props in favor of context, which can cause runtime issues if any consumers weren’t wrapped by
WidgetProvideror if defaults/derived values differ. Also changes chain filter render logic, which could alter visible wallet filtering behavior.Overview
Moves non-functional inputs to
WidgetContext.Root,Login,ConnectWallet, and related subcomponents now readappLogo,walletRegistry,chainNamespaces,deviceDetails, anduiConfigviauseWidget, dropping these from component props and simplifyingWidget→Rootwiring.Updates UI config plumbing and defaults.
WidgetContextnow storesuiConfigasLoginModalPropsand applies a defaultbuttonRadiusType: "pill";BottomSheetno longer takesuiConfigand instead acceptsborderRadiusTypedirectly.Behavioral tweak.
ConnectWalletChainFilternow derives namespaces from context and changes when the filter renders (early-return logic), which may affect chain selection UI/filters.Written by Cursor Bugbot for commit 57e7227. This will update automatically on new commits. Configure here.