kredits/kredits-web
9
0
Fork 0
Code Issues 22 Pull Requests 1 Actions Releases 18 Activity

Ask user to unlock their account #23

Merged
galfert merged 2 commits from feature/22-handle_locked_metamask into master 2017-06-14 20:15:18 +00:00
Conversation 11 Commits 2 Files Changed 1
+14
galfert commented 2017-06-14 00:35:52 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Closes #22

This asks the user to unlock their account when loading the app.

Do you think this is good enough? I'm thinking it might be better to only ask when it's actually required (e.g. for voting), but I'm not sure how much more effort that would be.

Closes #22 This asks the user to unlock their account when loading the app. Do you think this is good enough? I'm thinking it might be better to only ask when it's actually required (e.g. for voting), but I'm not sure how much more effort that would be.
raucao commented 2017-06-14 05:44:37 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Very cool! Works like a charm.

I edited the message a little bit, so it's easier to understand.

Very cool! Works like a charm. I edited the message a little bit, so it's easier to understand.
raucao (Migrated from github.com) reviewed 2017-06-14 05:44:51 +00:00
app/routes/index.js
@@ -8,0 +8,4 @@
beforeModel(transition) {
const kredits = this.get('kredits');
if (kredits.get('web3') && kredits.get('web3Provided')) {
raucao (Migrated from github.com) commented 2017-06-14 05:21:06 +00:00
Copy Link
Copy Source

Wouldn't the second condition be enough here?

Wouldn't the second condition be enough here?
raucao commented 2017-06-14 05:45:49 +00:00 (Migrated from github.com)
Copy Link
Copy Source

... feel free to merge, if the answer to my question is "no". ;)

... feel free to merge, if the answer to my question is "no". ;)
bumi commented 2017-06-14 08:09:41 +00:00 (Migrated from github.com)
Copy Link
Copy Source

The confirm blocks the user from actually visiting the page, right?
Wouldn't it be nicer to just display some kind of flash message which allows the user to still see the page?

The `confirm` blocks the user from actually visiting the page, right? Wouldn't it be nicer to just display some kind of flash message which allows the user to still see the page?
galfert (Migrated from github.com) reviewed 2017-06-14 10:02:42 +00:00
app/routes/index.js
@@ -8,0 +8,4 @@
beforeModel(transition) {
const kredits = this.get('kredits');
if (kredits.get('web3') && kredits.get('web3Provided')) {
galfert (Migrated from github.com) commented 2017-06-14 10:02:42 +00:00
Copy Link
Copy Source

No, because the web3 property actually checks if there is a user provided web3 when called for the first time. And it's not being called before this, so we have to access it first.

No, because the `web3` property actually checks if there is a user provided web3 when called for the first time. And it's not being called before this, so we have to access it first.
galfert commented 2017-06-14 10:04:44 +00:00 (Migrated from github.com)
Copy Link
Copy Source

The confirm blocks the user from actually visiting the page, right?
Wouldn't it be nicer to just display some kind of flash message which allows the user to still see the page?

That's why I used confirm. So the user can click "cancel" and still see the page. Also it's only shown if the user actually uses something like Metamask, but not if you don't have any extension installed.

> The confirm blocks the user from actually visiting the page, right? > Wouldn't it be nicer to just display some kind of flash message which allows the user to still see the page? That's why I used confirm. So the user can click "cancel" and still see the page. Also it's only shown if the user actually uses something like Metamask, but not if you don't have any extension installed.
raucao commented 2017-06-14 20:15:14 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Yes, clicking cancel works just fine, and one can also unlock the account while the dialog is open.

Yes, clicking cancel works just fine, and one can also unlock the account while the dialog is open.
raucao commented 2017-06-14 20:16:23 +00:00 (Migrated from github.com)
Copy Link
Copy Source

It'd be useful if core contributors would add the kredits labels themselves. It didn't occur to me that I had to check again, because you set it for your other PR. Not sure how to best solve this situation in the future.

It'd be useful if core contributors would add the kredits labels themselves. It didn't occur to me that I had to check again, because you set it for your other PR. Not sure how to best solve this situation in the future.
fsmanuel commented 2017-06-14 23:33:54 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Sorry, I'm late to the party but why don't you return a promise in the model hook that gets resolved on window.load that does all the setup work and returns the RSVP.hash?

Sorry, I'm late to the party but why don't you return a promise in the `model` hook that gets resolved on `window.load` that does all the setup work and returns the `RSVP.hash`?
raucao commented 2017-06-15 16:12:30 +00:00 (Migrated from github.com)
Copy Link
Copy Source

@fsmanuel Do you mean so that the rest can load in the background while unlocking accounts?

As this is merged already, a new PR would be most appreciated! 😉

@fsmanuel Do you mean so that the rest can load in the background while unlocking accounts? As this is merged already, a new PR would be most appreciated! 😉
😆 1
fsmanuel commented 2017-06-16 14:10:15 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Yep, that was the idea. Will see if I get free cycles for a PR.

Yep, that was the idea. Will see if I get free cycles for a PR.
Sign in to join this conversation.
Reviewers
No Reviewers
raucao
galfert
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#23
Delete Branch "feature/22-handle_locked_metamask"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?