Skip to content
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

feat(core,extension): LW-9743 revamp "Save your recovery phrase" screen #919

Closed
wants to merge 2 commits into from

Conversation

szymonmaslowski
Copy link
Contributor

@szymonmaslowski szymonmaslowski commented Feb 26, 2024

Checklist

  • JIRA - LW-9743
  • Proper tests implemented
  • Screenshots added.

Proposed solution

  • Changed the "Save your recovery screen" to have a single step and all 24 mnemonics visible.
  • Broken the screen about entering the mnemonic, but that will be solved by Shawn in subsequent PR which probably needs to be merged into this one.
  • Removed the screen preceding the mnemonic, the one having video, and moved it to the modal opened by clicking on the "Watch video" text.

Testing

Just go with the onboarding

Screenshots

image image

@szymonmaslowski szymonmaslowski requested a review from a team as a February 26, 2024 14:01
@github-actions github-actions bot added the browser Changes to the browser application. label Feb 26, 2024
@szymonmaslowski szymonmaslowski self-assigned this Feb 26, 2024
@tomislavhoracek tomislavhoracek force-pushed the LW-9170-multi-wallet-integration branch from daa2543 to e4ed066 Compare February 27, 2024 08:57
@szymonmaslowski szymonmaslowski force-pushed the feat/LW-9743-read-only-mnemonic branch from 8d750a3 to 7601001 Compare February 27, 2024 12:51
@szymonmaslowski szymonmaslowski changed the base branch from LW-9170-multi-wallet-integration to main February 27, 2024 12:52
Copy link

sonarcloud bot commented Feb 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
22.6% Duplication on New Code (required ≤ 3%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

Copy link

Allure report

allure-report-publisher generated test report!

smokeTests: ❌ test report for 7601001f

passed failed skipped flaky total result
Total 31 1 0 0 32

<MnemonicVideoPopupContent
translations={mnemonicVideoPopupContentTranslations}
onClickVideo={() => {
// TODO: https://input-output.atlassian.net/browse/LW-9761 handle analytics here based on the stage argument
Copy link
Contributor

Choose a reason for hiding this comment

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

Very good adding TODOs with a JIRA ticket 💪


.title {
@include text-heading;
color: var(--text-color-primary, #3d3b39);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if we need #3d3b39 as a fallback. I mean, if we lack --text-color-primary we would not notice it.

position: relative;
height: 272px;
border-radius: 16px;
border: 1px solid #efefef;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we don't use --text-color-secondary or similar instead of #efefef?

const [mnemonicStage, setMnemonicStage] = useState<MnemonicStage>('writedown');
const [mnemonicConfirm, setMnemonicWordsConfirm] = useState(initialMnemonicWordsConfirm);
const [isWriting, setIsWriting] = useState(false);
const [videoModalOpen, setVideoModalOpen] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

consider whether the mnemonicStage, videoModalOpen, and isWriting states could be consolidated or managed with a single state variable. This would reduce IMO complexity.

const [formState, setFormState] = useState({
  stage: 'writedown',
  isVideoModalOpen: false,
  isWriting: false
});

Also use is/has/contains/can for videoModalOpen (old discussion 😂)

{ key: WalletTimelineSteps.ALL_DONE, name: i18n.t('package.core.walletSetupStep.allDone') }
];

const walletSteps = isHardwareWallet ? hardwareWalletSteps : inMemoryWalletSteps;
Copy link
Contributor

Choose a reason for hiding this comment

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

  const walletSteps = isHardwareWallet
    ? [
        { key: WalletTimelineSteps.LEGAL_AND_ANALYTICS, name: i18n.t('package.core.walletSetupStep.legalAndAnalytics') },
        { key: WalletTimelineSteps.CONNECT_WALLET, name: i18n.t('package.core.walletSetupStep.connectWallet') },
        { key: WalletTimelineSteps.NAME_WALLET, name: i18n.t('package.core.walletSetupStep.nameWallet') },
        { key: WalletTimelineSteps.ALL_DONE, name: i18n.t('package.core.walletSetupStep.allDone') }
      ]
    : [
        { key: WalletTimelineSteps.LEGAL_AND_ANALYTICS, name: i18n.t('package.core.walletSetupStep.legalAndAnalytics') },
        { key: WalletTimelineSteps.WALLET_SETUP, name: i18n.t('package.core.walletSetupStep.walletSetup') },
        { key: WalletTimelineSteps.RECOVERY_PHRASE, name: i18n.t('package.core.walletSetupStep.recoveryPhrase') },
        { key: WalletTimelineSteps.ALL_DONE, name: i18n.t('package.core.walletSetupStep.allDone') }
      ];

@szymonmaslowski szymonmaslowski deleted the feat/LW-9743-read-only-mnemonic branch May 21, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser Changes to the browser application.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants