Refactor: Route children for welcome screens
#468
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #468 +/- ##
========================================
+ Coverage 9.33% 9.34% +0.01%
========================================
Files 114 112 -2
Lines 5172 5166 -6
Branches 699 698 -1
========================================
Hits 483 483
+ Misses 4025 4020 -5
+ Partials 664 663 -1 ☔ View full report in Codecov by Sentry. |
tuliomir
requested review from
r4mmer
and removed request for
pedroferreira1
| } | ||
|
|
||
| render() { |
There was a problem hiding this comment.
tip: To review these changes to the render() method I suggest using the "Hide whitespace" feature on GitHub, as it highlights only the words that were changed on this file, and not the indentation adjustments that were necessary:
There was a problem hiding this comment.
There are some problems to verify some screens since they only come up in special circunstances. For instance failing a request to the fullnode (or timing out) with the wallet locked should show the Server screen so the user can select another server with the wallet still locked, this is very hard to check without manually spoofing a request.
Maybe we should check these unlikely scenarios on this PR since we are changing the component that handles errors.
The state was not being fetched by the modal "hide" event listener. It was removed because it's no longer necessary.
|
@r4mmer : I had already tested turning off the internet connection and the The solution for this was to remove the This fix was implemented on f2cd753. |
| $('#requestErrorModal').modal('hide'); | ||
| this.props.history.push('/server/'); | ||
| const handleChangeServer = () => { | ||
| $(MODAL_DOM_ID).modal('hide'); |
There was a problem hiding this comment.
We should create an issue to convert this to the GlobalModal syntax
There was a problem hiding this comment.
Opened this issue on #471
src/components/BackButton.js
Outdated
| @@ -7,33 +7,33 @@ | |||
|
|
|||
| import React from 'react'; | |||
| import { t } from 'ttag'; | |||
| import { useHistory } from "react-router-dom"; | |||
There was a problem hiding this comment.
| import { useHistory } from "react-router-dom"; | |
| import { useHistory } from 'react-router-dom'; |
src/components/RequestError.js
Outdated
| import hathorLib from '@hathor/wallet-lib'; | ||
| import wallet from '../utils/wallet'; | ||
| import { useHistory } from "react-router-dom"; |
There was a problem hiding this comment.
| import { useHistory } from "react-router-dom"; | |
| import { useHistory } from 'react-router-dom'; |
src/screens/SentryPermission.js
Outdated
| @@ -5,7 +5,8 @@ | |||
| * LICENSE file in the root directory of this source tree. | |||
| */ | |||
|
|
|||
| import React from 'react'; | |||
| import React, { useState, useEffect } from 'react'; | |||
| import { useHistory } from "react-router-dom"; | |||
There was a problem hiding this comment.
| import { useHistory } from "react-router-dom"; | |
| import { useHistory } from 'react-router-dom'; |
Among the necessary steps to upgrade to React Router v6 are:
childrenprop instead ofcomponentorrenderon the<Route />elementsThere are many screens to update and the ones selected for the scope of this PR are only the screens that are accessed without any router wrappers on the
App.jsfile ( Ex.:NavigationRouteorStartedRoute).Some other components that are easy to refactor were also included, such as the
BackButtonthat no longer needs anypropsfrom its parent.Acceptance Criteria
Notes
As the screens' translatable messages were moved around, the
locale/texts.pothad to be updated. This file alone has over 1000 lines of changed code that are actually from other contexts too.Security Checklist