(Wallet) Tweak onboarding navigation controls #21673

simoarpe · 2024-01-22T14:01:30Z

Resolves brave/brave-browser#35488

Improves onboarding navigation controls by adding close icon where appropriate.
Implements the option to navigate back to the previous screen where appropriate.
Fixes an issue where a user could navigate back to the Wallet creation screen, remaining stuck.
Performs refactoring to OnNextPage interface.
Implements new base abstract class BaseWalletNetPageFragment.
Perform refactoring to CryptoOnboardingFragment.
Rename package from org.chromium.chrome.browser.crypto_wallet.fragments.onboarding_fragments to org.chromium.chrome.browser.crypto_wallet.fragments.onboarding.

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

⚠ Note for QA: the goal of the plan is to verify close and back buttons. The onboarding flow may slightly differ when testing will happen.

Reset and clear any pre-existing Wallet data, if any.
Navigate to Wallet section.
Tap on Get Started.
Tap on back icon and observe back action works as expected.
Navigate to Wallet section.
Tap on Get Started.
Tap on close icon and observe close action works as expected.
Navigate to Wallet section.
Tap on Get Started.
Type a new password, confirm it and tap on continue.
Observe the navigation icon is not present on the next screen.
Check the required terms and tap Continue.
Observe the next screen allows backward navigation.
Tap on close icon and observe close action works as expected.
Reset and clear Wallet data.
Navigate to Wallet section.
Tap on Restore.
Tap on back icon and observe back action works as expected.

…age interface

github-actions · 2024-01-23T10:24:46Z

[puLL-Merge] - brave/brave-core@21673

Description

The pull request contains a set of changes aimed at restructuring the onboarding flow for the Brave Wallet feature within the Brave browser for Android. The changes suggest a refactor of fragment classes used during the crypto wallet setup, including renaming of classes and files also modifying the flow of user navigation through the onboarding process. Additionally, unused static variables from the Utils class have been removed.

Changes

Changes

Changes organized by filename:

android/brave_java_sources.gni

Added BaseWalletNextPageFragment.java and UnlockWalletFragment.java to the compilation source list.
Renamed several onboarding-related Java files to a new naming convention (e.g., BackupWalletFragment.java to OnboardingBackupWalletFragment.java).
Deleted paths for old onboarding fragments and added paths for new onboarding classes.

BraveWalletActivity.java

Import statements updated to match the renamed files.
The deprecated static variables have been removed, and instead, a WalletAction enum within BraveWalletActivity handles different wallet actions.
Changes made to the logic of setting up navigation fragments based on the new WalletAction enum.
Simplified the setNavigationFragments and replaceNavigationFragments methods as per the new fragment classes' names and behavior.

CryptoWalletOnboardingPagerAdapter.java

Variable name mNavigationItems has been corrected from navigationItems to follow Java naming conventions.
Adapted replaceWithNavigationItems method to work with the new list of NavigationItems.

RecoveryPhraseAdapter.java

Updated to match with the renaming of VerifyRecoveryPhraseFragment to OnboardingVerifyRecoveryPhraseFragment.
Private fields are named appropriately with m prefix following standard Java naming conventions.

BaseWalletNextPageFragment.java (New file)

This new base fragment class is created to be extended by wallet and onboarding fragments, encapsulating common functionality such as extracting the OnNextPage interface.

UnlockWalletFragment.java

Removed deprecated imports that are no longer necessary due to renaming.
Updated methods and logic to match the refactored structure and naming.

Onboarding Fragments (New files)

New onboarding fragment classes introduced such as BaseOnboardingWalletFragment, OnboardingBackupWalletFragment, etc., to encapsulate the logic and maintain consistency.

Utils.java

Removed deprecated static integer variables, as they are no longer necessary with the implementation of the WalletAction enum.
Updated the isBiometricAvailable method to check for biometric support without requiring an API level check for R.

activity_brave_wallet.xml

Added a close button (onboarding_close_button) and a back button (onboarding_back_button) to the wallet activity layout.

brave_colors.xml (both day and night)

Added new color resources for onboarding item background color.

Security Hotspots

BraveWalletActivity.java: - Ensure that all navigation and fragment transaction actions are secure and do not expose any sensitive information or lead to unauthorized operations.
Utils.java: - Make sure the removal of the static variable does not cause any previously handled security checks to fail.
BaseWalletNextPageFragment.java & Onboarding Fragments: - Verify that the newly introduced base classes and refactored onboarding fragments maintain the app's security standards, including proper handling of user input and credential management.
UnlockWalletFragment.java: - When implementing biometric prompt and password-handling related changes, ensure that there are no security vulnerabilities in terms of how sensitive data is stored or processed.

simoarpe · 2024-01-23T13:22:06Z

android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java

-        mCryptoWalletOnboardingViewPager.addOnPageChangeListener(
-                new ViewPager.OnPageChangeListener() {
-                    @Override
-                    public void onPageScrolled(
-                            int position, float positionOffset, int positionOffsetPixels) {}
-
-                    @Override
-                    public void onPageSelected(int position) {
-                        if (position == 0 || position == 2) {
-                            onboardingBackButton.setVisibility(View.GONE);
-                        } else {
-                            onboardingBackButton.setVisibility(View.VISIBLE);
-                        }


This logic is not required anymore as now each fragment can be individually configured overriding the abstract methods canBeClosed, canNavigateBack.

simoarpe · 2024-01-23T13:24:06Z

android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java

@@ -281,7 +282,55 @@ private void setNavigationFragments(int type) {
        addRemoveSecureFlag(true);
    }

-    private void addRemoveSecureFlag(boolean add) {
+    private void replaceNavigationFragments(@NonNull final WalletAction walletAction) {


This method has been moved below setNavigationFragments but remains unchanged.

simoarpe · 2024-01-23T13:26:59Z

android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java

@@ -417,49 +417,71 @@ public boolean showBiometricPrompt() {
    }

    @Override
-    public void showBiometricPrompt(boolean show) {
-        mShowBiometricPrompt = show;
+    public void enableBiometricPrompt() {


Renamed showBiometricPrompt to enableBiometricPrompt.
Removed boolean from signature as always true.

simoarpe · 2024-01-23T13:33:26Z

...omium/chrome/browser/crypto_wallet/fragments/onboarding/OnboardingRestoreWalletFragment.java

-    public static RestoreWalletFragment newInstance(boolean isOnboarding) {
-        RestoreWalletFragment fragment = new RestoreWalletFragment();
-        Bundle args = new Bundle();
-        args.putBoolean(IS_ONBOARDING, isOnboarding);
-        fragment.setArguments(args);
-        return fragment;
-    }


Removed arguments from RestoreWalletFragment instance creation because unused.

simoarpe · 2024-01-23T13:38:16Z

...um/chrome/browser/crypto_wallet/fragments/onboarding_fragments/CryptoOnboardingFragment.java

-
-import org.chromium.chrome.browser.crypto_wallet.listeners.OnNextPage;
-
-public class CryptoOnboardingFragment extends Fragment {


Fragment renamed to BaseOnboardingWalletFragment.

simoarpe · 2024-01-23T13:42:30Z

android/java/org/chromium/chrome/browser/crypto_wallet/util/Utils.java

            return biometricManager.canAuthenticate() == BiometricManager.BIOMETRIC_SUCCESS;
        } else {
            // For API level < Q, we will use FingerprintManagerCompat to check enrolled
            // fingerprints. Note that for API level lower than 23, FingerprintManagerCompat behaves
            // like no fingerprint hardware and no enrolled fingerprints.
            FingerprintManagerCompat fingerprintManager = FingerprintManagerCompat.from(context);
-            return fingerprintManager != null && fingerprintManager.isHardwareDetected()
+            return fingerprintManager.isHardwareDetected()


Optimized check as fingerprintManager is always not null.

android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java

android/java/org/chromium/chrome/browser/crypto_wallet/adapters/RecoveryPhraseAdapter.java

...chromium/chrome/browser/crypto_wallet/fragments/onboarding/BaseOnboardingWalletFragment.java

github-actions · 2024-01-23T15:59:47Z

The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "password" and so security team members have been added as reviewers to take a look.

No need to request a full security review at this stage, the security team will take a look shortly and either clear the label or request more information/changes.

Notifications have already been sent, but if this is blocking your merge feel free to reach out directly to the security team on Slack so that we can expedite this check.

SergeyZhukovsky

++

simoarpe added CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-ios Do not run CI builds for iOS CI/skip-windows-x64 Do not run CI builds for Windows x64 labels Jan 22, 2024

simoarpe self-assigned this Jan 22, 2024

github-actions bot added feature/web3/wallet puLL-Merge labels Jan 22, 2024

simoarpe added 10 commits January 22, 2024 16:29

Perform minor improvements

c06871a

Add back and close icon to main layout

e39bef2

Perform refactoring to OnNextPage interface

a4dddc6

Implement BaseWalletNextPageFragment that holds a reference to next p…

2852554

…age interface

Refine Utils class

a951ed3

Switch Wallet actions from int to enum type

18c5dbb

Move UnlockWalletFragment outside onboarding package

07abca0

Rename package

e40bf2a

Apply code formatting

67b35e3

Rename onboarding fragments

58588fc

simoarpe force-pushed the simone/new-onboarding-navigation-logic branch from d1d241f to 58588fc Compare January 22, 2024 15:29

simoarpe added 3 commits January 22, 2024 17:28

Add Javadocs and code comments

f0a7caa

Apply code formatting

9bfb9cb

Fix presubmit warnings & errors

131184e

simoarpe added 3 commits January 23, 2024 11:26

Fix further presubmit warnings

c355382

Apply code formatting

3138ac4

Rename method

1e19dab

simoarpe commented Jan 23, 2024

View reviewed changes

Rename method

65daca6

simoarpe commented Jan 23, 2024

View reviewed changes

simoarpe requested a review from SergeyZhukovsky January 23, 2024 13:42

SergeyZhukovsky reviewed Jan 23, 2024

View reviewed changes

android/java/org/chromium/chrome/browser/crypto_wallet/activities/BraveWalletActivity.java Outdated Show resolved Hide resolved

SergeyZhukovsky reviewed Jan 23, 2024

View reviewed changes

android/java/org/chromium/chrome/browser/crypto_wallet/adapters/RecoveryPhraseAdapter.java Outdated Show resolved Hide resolved

SergeyZhukovsky reviewed Jan 23, 2024

View reviewed changes

...chromium/chrome/browser/crypto_wallet/fragments/onboarding/BaseOnboardingWalletFragment.java Outdated Show resolved Hide resolved

Address review feedback

7fd524e

github-actions bot added the needs-security-review label Jan 23, 2024

github-actions bot assigned fmarier and thypon Jan 23, 2024

Change method visibility

99f91fb

SergeyZhukovsky approved these changes Jan 23, 2024

View reviewed changes

fmarier removed the needs-security-review label Jan 23, 2024

fmarier unassigned fmarier and thypon Jan 23, 2024

simoarpe merged commit 9dc756f into master Jan 24, 2024

simoarpe deleted the simone/new-onboarding-navigation-logic branch January 24, 2024 10:13

github-actions bot added this to the 1.64.x - Nightly milestone Jan 24, 2024

srirambv mentioned this pull request Mar 11, 2024

Tweak navigation controls for new onboarding UI brave/brave-browser#35488

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Wallet) Tweak onboarding navigation controls #21673

(Wallet) Tweak onboarding navigation controls #21673

simoarpe commented Jan 22, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024

Changes

android/brave_java_sources.gni

BraveWalletActivity.java

CryptoWalletOnboardingPagerAdapter.java

RecoveryPhraseAdapter.java

BaseWalletNextPageFragment.java (New file)

UnlockWalletFragment.java

Onboarding Fragments (New files)

Utils.java

activity_brave_wallet.xml

brave_colors.xml (both day and night)

simoarpe Jan 23, 2024

simoarpe Jan 23, 2024

simoarpe Jan 23, 2024

simoarpe Jan 23, 2024

simoarpe Jan 23, 2024

simoarpe Jan 23, 2024

github-actions bot commented Jan 23, 2024

SergeyZhukovsky left a comment


		import org.chromium.chrome.browser.crypto_wallet.listeners.OnNextPage;

		public class CryptoOnboardingFragment extends Fragment {

(Wallet) Tweak onboarding navigation controls #21673

(Wallet) Tweak onboarding navigation controls #21673

Conversation

simoarpe commented Jan 22, 2024 • edited Loading

Resolves brave/brave-browser#35488

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

github-actions bot commented Jan 23, 2024

Description

Changes

android/brave_java_sources.gni

BraveWalletActivity.java

CryptoWalletOnboardingPagerAdapter.java

RecoveryPhraseAdapter.java

BaseWalletNextPageFragment.java (New file)

UnlockWalletFragment.java

Onboarding Fragments (New files)

Utils.java

activity_brave_wallet.xml

brave_colors.xml (both day and night)

Security Hotspots

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

simoarpe Jan 23, 2024

Choose a reason for hiding this comment

github-actions bot commented Jan 23, 2024

SergeyZhukovsky left a comment

Choose a reason for hiding this comment

simoarpe commented Jan 22, 2024 •

edited

Loading