Skip to content

Conversation

@ieow
Copy link

@ieow ieow commented May 14, 2025

Description

1.4
Update New SRP onboarding flow as per figma design
https://www.figma.com/design/pViOUcmjwhEzFsdrwknpNc/Onboarding-redesign?node-id=434-55200&t=wmEoQSF9g9lFtkwp-0

This PR is part 4 of 7 for the new SRP ui update

This PR cover

  • new ui flow for import srp
Screen.Recording.2025-05-21.at.10.18.02.AM.mov

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

ieow and others added 4 commits May 9, 2025 18:06
Co-authored-by: smgv <gpatra1996@gmail.com>
Co-authored-by: smgv <gpatra1996@gmail.com>
Co-authored-by: smgv <gpatra1996@gmail.com>
@github-actions
Copy link

github-actions bot commented May 14, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@ieow ieow mentioned this pull request May 14, 2025
7 tasks
@smgv
Copy link

smgv commented May 15, 2025

I have read the CLA Document and I hereby sign the CLA

@ieow ieow marked this pull request as ready for review May 21, 2025 14:16
@ieow ieow force-pushed the feat/new-srp-welcom branch from 2a3adb3 to fc8632c Compare May 23, 2025 06:51
@ieow ieow force-pushed the feat/new-srp-welcom branch from fc8632c to 756db9a Compare May 23, 2025 08:59
</View>
)
: null,
// headerBackTitle: strings('navigation.back'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this prop missing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is replace with headerLeft
the commented code is removed
e0de428abfefd66b113548edd94235190d134566

const innerStyles = StyleSheet.create({
headerStyle: {
backgroundColor: themeColors.background.default,
backgroundColor: color || themeColors.background.default,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use a lot of the name color in the params and we only use it for the backgroundColor, would be more accurate to rename it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced with headerBgColor
e0de428

const innerStyles = StyleSheet.create({
headerStyle: {
backgroundColor: themeColors.background.default,
backgroundColor: color || themeColors.background.default,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use a lot of the name color in the params and we only use it for the backgroundColor, would be more accurate to rename it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced with headerBgColor
e0de428

StyleSheet.create({
scroll: {
flex: 1,
// marginTop: 100,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgotten comment?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commented code is removed here
e0de428

import Routes from '../../../constants/navigation/Routes';
import { selectAccounts } from '../../../selectors/accountTrackerController';
import trackOnboarding from '../../../util/metrics/TrackOnboarding/trackOnboarding';
import { trace, TraceName, TraceOperation } from '../../../util/trace';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch this seemed to be forgotten here, thank you!

);
};

const termsOfUse = async () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something doesn't add up, this PR have the same changes of this one although the branch of this PR is branching off it!

Can we solve this issue to facilitate review? This will make it a bit harder to understand the final result of the code

BVLinearGradient: cb006ba232a1f3e4f341bb62c42d1098c284da70
CocoaAsyncSocket: 065fd1e645c7abab64f7a6a2007a48038fdc6a99
DoubleConversion: f16ae600a246532c4020132d54af21d0ddb2a385
DoubleConversion: 5189b271737e1565bdce30deb4a08d647e3f5f54

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems not necessary

@ieow
Copy link
Author

ieow commented May 24, 2025

This pr is replaced with new pr #53 as request to get the PR's unit tests pass

Most of the comments are addressed in the #53 commit
e0de428

Closing this pr in favour or pr #53 , #54 , #55

@ieow ieow closed this May 24, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants