Switch the network if removing the active network #566

Nasicus · 2018-02-14T12:00:08Z

Also fixes a bug related to the background.

dependencies.min.js:1 TypeError: Cannot read property 'match' of undefined
    at scope.$watch (background-style.directive.js:40)
    at Scope.$digest (dependencies.min.js:1)
    at Scope.$apply (dependencies.min.js:1)
    at dependencies.min.js:1
    at Object.invoke (dependencies.min.js:1)
    at doBootstrap (dependencies.min.js:1)
    at bootstrap (dependencies.min.js:1)
    at angularInit (dependencies.min.js:1)
    at dependencies.min.js:1
    at HTMLDocument.trigger (dependencies.min.js:1)

Fixes: #565

Nasicus · 2018-02-15T10:08:56Z

added one more commit which fixes: #568

alexbarnsley

Typo change mostly. Possible UX change.
Everything else works well and didn't have any actual problems 👌

alexbarnsley · 2018-02-19T10:48:53Z

client/app/src/accounts/account.controller.js

+          if (isActive) {
+            dialogService.openLoadingDialog(self.currentTheme,
+                                            gettext('Network removed succesfully'),
+                                            gettext('Network removed succesfully - will now switch the network'))


"successfully" is spelt incorrectly. Also, i'm not sure how much work it would be, but maybe changing it so it's not a loading dialog but a dialog that tells them the wallet needs to reload, with a reload button. then it just reloads on close 🤷‍♂️

Well "the problem" is, that this is a generic loading dialog which I did either reuse here or reuse in another place (cannot remember) :P For me in this situation it's enough (because almost nobody ever deletes a network) - but of course in the end it's your decision :)

@luciorubeens and/or @j-a-m-l, what do you guys think?

A simple solution would be adding a new parameter to openLoadingDialog that permits customizing its behaviour or content.

2 examples:

Using templateUrl, to add there the reload button.

Passing a options parameter that enables HTML instead of pure text (and change the template too).

Those ideas could be used to display the success removal message and the reload button, as a confirmation, instead of using the toast and the timeout.

I guess I just fail to see the advantage of showing a button to the user. He cannot do anything anyway - he HAS to reload because the network was deleted and the dialog is modal.

To me the UX didn't feel seamless. I'd rather a button I have to press to reload, because then I can press it in my own time. Loading screens I feel are used for processing in the background or waiting for something to happen, and not to display a message to the user, because I might be a slow reader who didn't manage to read the whole message before it disappeared.

Okay, that's a valid point.
I will adjust it accordingly.
However I will open a custom dialog then, because the loading indicator is in this case not relevant anymore, you don't have to wait for anything anyway :)

Done - what do you think?

Looks good! I'll have a cheeky test Monday

alexbarnsley · 2018-02-19T10:48:56Z

client/app/src/accounts/account.controller.js

-          toastService.success(gettext('Network removed succesfully!'), 3000)
+          if (isActive) {
+            dialogService.openLoadingDialog(self.currentTheme,
+                                            gettext('Network removed succesfully'),


"successfully" is spelt incorrectly

alexbarnsley · 2018-02-19T10:49:05Z

client/app/src/accounts/account.controller.js

+            setTimeout(() => networkService.switchNetwork(null, true), 4000)
+          } else {
+            self.listNetworks = networkService.getNetworks()
+            toastService.success(gettext('Network removed succesfully!'), 3000)


"successfully" is spelt incorrectly

alexbarnsley · 2018-02-19T10:50:36Z

client/app/src/accounts/account.controller.js

+            dialogService.openLoadingDialog(self.currentTheme,
+                                            gettext('Network removed succesfully'),
+                                            gettext('Network removed succesfully - will now switch the network'))
+            setTimeout(() => networkService.switchNetwork(null, true), 4000)


Maybe halve the timeout as felt a bit long for me, it was enough time for me to wonder if it was going to reload.

I set the timeout this high because I thought else the user doesn't have enough time to read the message, but I will halve it in this case.
Edit: Tested it and with 2000 I find it hard to read, set it to 3000 - so we meet in the middle :D

oh maybe take a look at my other comment. instead of doing a timeout, #566 (comment)

alexbarnsley · 2018-02-26T11:57:57Z

Looks good and working as expected. 👍

Nasicus added 3 commits February 14, 2018 12:21

switch the network if removing the active network

bb32a24

use default background for new networks (gave an exception)

5fa9a6a

set active network as selected in manage networks

9cf9098

alexbarnsley suggested changes Feb 19, 2018

View reviewed changes

Nasicus added 3 commits February 19, 2018 12:23

cr changes

237aecd

show a confirm dialog instead of switching automatically

2910b3c

adjust messages

65b6222

alexbarnsley merged commit e5eac4a into ArkEcosystem:master Feb 26, 2018

Nasicus deleted the feat/switch-network-on-remove branch February 26, 2018 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch the network if removing the active network #566

Switch the network if removing the active network #566

Nasicus commented Feb 14, 2018 •

edited

Nasicus commented Feb 15, 2018

alexbarnsley left a comment

alexbarnsley Feb 19, 2018

Nasicus Feb 19, 2018 •

edited

alexbarnsley Feb 19, 2018

j-a-m-l Feb 23, 2018 •

edited

Nasicus Feb 24, 2018

alexbarnsley Feb 24, 2018

Nasicus Feb 24, 2018 •

edited

Nasicus Feb 24, 2018

alexbarnsley Feb 24, 2018

alexbarnsley Feb 19, 2018

Nasicus Feb 19, 2018

alexbarnsley Feb 19, 2018

Nasicus Feb 19, 2018

alexbarnsley Feb 19, 2018

Nasicus Feb 19, 2018 •

edited

alexbarnsley Feb 19, 2018

alexbarnsley commented Feb 26, 2018

Switch the network if removing the active network #566

Switch the network if removing the active network #566

Conversation

Nasicus commented Feb 14, 2018 • edited

Nasicus commented Feb 15, 2018

alexbarnsley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nasicus Feb 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-a-m-l Feb 23, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nasicus Feb 24, 2018 • edited

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

Nasicus Feb 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexbarnsley commented Feb 26, 2018

Nasicus commented Feb 14, 2018 •

edited

Nasicus Feb 19, 2018 •

edited

j-a-m-l Feb 23, 2018 •

edited

Nasicus Feb 24, 2018 •

edited

Nasicus Feb 19, 2018 •

edited