-
Notifications
You must be signed in to change notification settings - Fork 323
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
β¨(llm/lld): add dynamic learn more for LS activation #7825
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
4 Skipped Deployments
|
7b9437d
to
15bc8ba
Compare
15bc8ba
to
bf5b91e
Compare
onPressLearnMore: () => void; | ||
hasLearnMoreLink?: boolean; |
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.
You can simplify just by onPressLearnMore
as optional ?
|
||
useInitMemberCredentials(); | ||
|
||
const onPressSyncAccounts = () => onSyncMethodPress(); | ||
|
||
const onPressHasAlreadyCreatedAKey = () => onSyncMethodPress(); | ||
|
||
const onPressLearnMore = () => learnMoreLink && Linking.openURL(learnMoreLink); |
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.
you may have an eslint problem here for no-unused-expressions
rule. For a better readability and code maintainability the better is to use an if bloc and invoke your openURL function there.
const learnMoreUrl = ledgerSyncFF?.params?.learnMoreLink; | ||
const hasLearnMoreLink = !!learnMoreUrl; | ||
|
||
const onLearnMore = () => hasLearnMoreLink && openURL(learnMoreUrl); |
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.
be38291
to
f400a03
Compare
Changed for |
8d2dd0d
f400a03
to
8d2dd0d
Compare
β Checklist
npx changeset
was attached.π Description
Add a field to the feature flag so we can dynamically change the learn more url when activate ledger sync.
The learmore link will appears only if a value is provided
The UI has changed on both LLD and LLM
Good and bad case integration tests added
The new field is
learnMoreLink
LLD :
https://github.com/user-attachments/assets/92fae350-6a76-4b0f-920c-efbfa6aeb3e9
LLM :
https://github.com/user-attachments/assets/bc0fec2d-8b05-4673-aa08-33feb6ec6eaf
β Context
π§ Checklist for the PR Reviewers