[DDW-423] Finish ETC code cleanup #1108
Conversation
nikolaglumac
changed the title
There was a problem hiding this comment.
Great work @daniloprates!
I have left you some comments and also I think we should reorganize some of the files - e.g. we still have ada subdirectory under source/renderer/app/actions and source/renderer/app/stores. Those should be eliminated / merged with other files from the parent directory. This will result in different structure - we will finally be able to get rid of the ada namespace and thus we will no longer have stores.ada.wallets but stores.wallets (should be the same for the actions)...
source/common/environment.js
Outdated
| @@ -24,7 +24,7 @@ const environment = Object.assign({ | |||
| TEST: 'test', | |||
| PRODUCTION: 'production', | |||
| NETWORK: process.env.NETWORK || 'development', | |||
| API: process.env.API || 'ada', | |||
| API: 'ada', | |||
There was a problem hiding this comment.
@daniloprates I think we can remove this line as it is no longer used in the code.
| !isConnected ? styles.connectingLogo : styles.syncingLogo, | ||
| ]); | ||
| const apiLogoStyles = classNames([ | ||
| styles[`${environment.API}-apiLogo`], | ||
| styles['ada-logo'], |
There was a problem hiding this comment.
@daniloprates here you used ada-logo and it should be ada-apiLogo...
| @@ -69,7 +68,7 @@ export default class TermsOfUseForm extends Component<Props, State> { | |||
| isSubmitting ? styles.submitButtonSpinning : styles.submitButton, | |||
| ]); | |||
|
|
|||
| const checkboxLabel = environment.isEtcApi() ? 'checkboxLabelWithDisclaimer' : 'checkboxLabel'; | |||
| const checkboxLabel = 'checkboxLabel'; | |||
There was a problem hiding this comment.
@daniloprates we don't need this variable any longer - we can now use directly messages. checkboxLabel
| } = environment; | ||
|
|
||
| const apiName = intl.formatMessage(environmentSpecificMessages[API].apiName); | ||
| const apiName = intl.formatMessage(environmentSpecificMessages.ada.apiName); |
There was a problem hiding this comment.
@daniloprates we no longer need environmentSpecificMessages as we are using only ADA now. Please move all those messages under globalMessages (which means they will still be in the same file but just in the default export).
|
@daniloprates I have added the |
|
@daniloprates there are some flow issues - can you please fix those? |
There was a problem hiding this comment.
@daniloprates can you please merge the Ada specific stores into the generic ones? We should no longer need to have one WalletStore and then AdaWalletStore which extends it. It should only be WalletStore which contains merged code from both of these. The same rule should be applied to the others stores as well. Thanks!
|
@daniloprates I have added some code style imporvements. The last part to be done in this PR is to merge the ADA specific stores to the generic ones (like stated in my review). |
|
@daniloprates I have added the |
…com/input-output-hk/daedalus into fix/ddw-423-finish-etc-code-cleanup
This PR finishes the ETC code cleanup
Note: The second commit removes the
isAdaApiand any other if the API is ADA.Review Checklist:
Basics
yarn run test)yarn run dev)yarn run package/ CI builds)yarn run flow:test)yarn run lint)yarn run manage:translationsproduces no changes)yarn run storybook)yarn.lockfile is updatedCode Quality
Testing
After Review:
doneon the Youtrack board