Implement routing for modals - Closes #554 #750

reyraa · 2017-09-18T07:09:12Z

Defines routing for dialogs
Creates Relativelink component
Edits dialog component to work independently from its callees.
replaces all buttons which trigger action to open a dialog with a relative link to the corresponding route

Closes #554
Closes #553

…tton

slaweet · 2017-09-22T08:10:22Z

package.json

@@ -32,6 +32,7 @@
    "bitcore-mnemonic": "=1.1.1",
    "copy-to-clipboard": "=3.0.6",
    "flexboxgrid": "=6.3.1",
+    "history": "^4.7.2",


This should use = instead of ^.
We should set up this in .npmrc file like lisk-js guys did LiskArchive/lisk-elements#382

TBH, I never liked this approach, choosing = we easily end up with outdated dependencies. if there's a breaking change in one of the dependencies, I prefer to get notified of somehow. that's exactly the reason I clean install my node modules every once in a while.

@reyraa, @slaweet https://github.com/greenkeeperio/greenkeeper

Sure it's a good idea, but please don't tell me to add it in this PR :-D

… store

slaweet · 2017-09-25T14:35:26Z

features/accountManagement.feature

+    And I select option no. 3 from "network" select
+    When I fill in "http://localhost:4000" to "address" field
+    When I fill in "wagon stock borrow episode laundry kitten salute link globe zero feed marble" to "passphrase" field
+    And I click "login button"


I think these changes are not necessary.

slaweet · 2017-09-25T14:48:59Z

src/components/header/index.test.js

      <I18nextProvider i18n={ i18n }>
        <HeaderHOC />
-      </I18nextProvider>
+      </I18nextProvider></Router>


I'd prefer <Router> on its own line

slaweet · 2017-09-25T14:53:03Z

src/components/login/login.js

-    });
-  }
+  // eslint-disable-next-line class-methods-use-this
+  getPassphraseValidationError(passphrase) {


functionality inside getPassphraseValidationError was moved to PassphraseInput component. Here it's reintroduced from a merging.

slaweet · 2017-09-25T15:05:18Z

src/components/tabs/tabs.js

+    {getTabs(isDelegate).map((tab, index) =>
+      <Tab
+        key={index}
+        label={t(tab)}


Calling t(tab) doesn't allow i18n-scanner to find tab names for the source*.json file.

slaweet · 2017-09-25T15:06:53Z

src/components/transactions/transactionType.js

      break;
    default:
      type = false;
      break;
  }
  const address = props.address !== props.senderId ? props.senderId : props.recipientId;
  const template = type ?
-    <span className={sytles.smallButton}>{type}</span> :
-    <ClickToSend recipient={address} className='from-to' >
+    <span className={styles.smallButton}>{t(type)}</span> :


Calling t(type) doesn't allow i18n-scanner to find type names for the source *.json file.

slaweet · 2017-09-25T15:09:43Z

src/utils/saveAccount.js

@@ -2,16 +2,17 @@ export const getSavedAccount = () => {
  const savedAccounts = localStorage.getItem('accounts');
  let account;
  if (savedAccounts) {
-    account = JSON.parse(savedAccounts)[0];
+    account = JSON.parse(savedAccounts);


I don't see why [0] was removed.

Since we aim to have multiple accounts there, I've implement the reducer to handle multiple. So I always should receive an array, even so currently the utility returns only one item.
There are also other problems with this utility which I found it irrelevant to this PR so I didn't touch.

then account should be accounts

There's a ticket to fix many other issues about this utility.

alepop · 2017-09-25T20:09:04Z

src/components/relativeLink/index.js

+  if (primary !== undefined) style += `${buttonStyle.primary} `;
+  if (disableWhenOffline !== undefined) style += `${offlineStyle.disableWhenOffline} `;
+  if (style !== '') style += ` ${buttonStyle.button}`;
+


I think that https://github.com/JedWatson/classnames is better solution for dynamic classnames

Seems a neat way to go, since we use string literal everywhere in the project, it's worth creating a ticket if we plan to use this.

alepop · 2017-09-25T20:12:14Z

src/components/saveAccount/saveAccount.js

+    const index = networksRaw.map((item, i) => {
+      return (item.name === network) ? i : null;
+    }).filter(item => item !== null)[0];
+    accountSaved({


.find(item => item ! == null); will return first element that not equal null

alepop · 2017-09-25T20:17:16Z

src/store/middlewares/savedAccounts.js

+  switch (action.type) {
+    case actionTypes.accountSaved:
+      store.dispatch(successToastDisplayed({ label: 'Account saved' }));
+      break;


why not use return instead of break in the switch case? It will look more compact.

There's a ticket to fix many other issues about this utility.

reyraa added 8 commits September 8, 2017 11:25

Create a list of dialogs config

8560255

Tweak tabs to consider dialogs in pathname

577c189

Show/hide dialogs with route change

9a3f4fe

Add optional paramter: dialog

caaae39

Create relative link element to handle dialog routes

ab9639c

Use routes to show/hide dialogs

37a2348

Create new account component as a wrapepr over passphrase

182a041

Use relative link instead of impiratively firing dialog actions

8c9be24

reyraa added the 🏗️ in progress label Sep 18, 2017

reyraa self-assigned this Sep 18, 2017

reyraa requested review from yasharAyari, slaweet and alepop September 18, 2017 07:09

reyraa force-pushed the 554-routing-for-modals branch from 4111f12 to 79bed75 Compare September 18, 2017 10:15

reyraa added 5 commits September 19, 2017 10:50

Merge conflicts

2bdb882

Replace action butto with relative link to vote route

22d7103

Rename receiveButton to receiveDialog since we no longer use it as bu…

adf48e6

…tton

Use ReceiveDialog directly though route change

e8d81f0

Show only confirmed votes in myVotes list

0924d89

reyraa force-pushed the 554-routing-for-modals branch from 79bed75 to 0924d89 Compare September 19, 2017 10:12

reyraa added 6 commits September 19, 2017 14:14

Remove Redux connect from ClickToSend since it is not required anymore

6a9619d

Remove cnnect from ClickToSend since we do not pass any params

537f40f

Remove ReceiveButton component since it is not needed anymore

36e60e5

Update unit tests to pass history to context

8894357

Add history package and use hashHistory

c53edd0

Adapt unit tests

04b72fb

reyraa added 2 commits September 21, 2017 12:57

Fix conflicts

42a2f88

Styling modifications

66bd091

reyraa force-pushed the 554-routing-for-modals branch from 59145e7 to 66bd091 Compare September 21, 2017 14:10

slaweet reviewed Sep 22, 2017

View reviewed changes

reyraa added 6 commits September 22, 2017 17:37

Update utility to always return array

6698fa2

Update actions, reducers and middlewares to maintain savedAccounts on…

1b0ff22

… store

Update savedAccounts compoonent and button to use data from store

6fa6045

Use relative links instead of buttons and update unit tests accordingly

47cbba1

Refine the name of actions

68ab05b

Adapt e2e tests

e016b65

Merge conflicts

302a807

reyraa added 👁️ pending review and removed 🏗️ in progress labels Sep 25, 2017

slaweet suggested changes Sep 25, 2017

View reviewed changes

Remove obsolete codes from login component, update the tests

aca5af9

alepop reviewed Sep 25, 2017

View reviewed changes

Use find instead of filter

64e0848

slaweet approved these changes Sep 26, 2017

View reviewed changes

reyraa merged commit 0c8a733 into development Sep 26, 2017

reyraa deleted the 554-routing-for-modals branch September 26, 2017 07:28

slaweet added ✅ ready and removed 👁️ pending review labels Sep 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement routing for modals - Closes #554 #750

Implement routing for modals - Closes #554 #750

reyraa commented Sep 18, 2017

slaweet Sep 22, 2017

reyraa Sep 25, 2017 •

edited

Loading

alepop Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 25, 2017

slaweet Sep 25, 2017

reyraa Sep 26, 2017

alepop Sep 25, 2017

reyraa Sep 26, 2017 •

edited

Loading

alepop Sep 25, 2017

alepop Sep 25, 2017

reyraa Sep 26, 2017

Implement routing for modals - Closes #554 #750

Implement routing for modals - Closes #554 #750

Conversation

reyraa commented Sep 18, 2017

Choose a reason for hiding this comment

reyraa Sep 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyraa Sep 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyraa Sep 25, 2017 •

edited

Loading

reyraa Sep 26, 2017 •

edited

Loading