Sign Up View #186
Sign Up View #186
Conversation
frontend/simple/views/NavBar.vue
Outdated
| login: async function () { | ||
| try { | ||
| let response = await namespace.lookup(this.name) | ||
| let identity = JSON.parse(response.text) |
There was a problem hiding this comment.
This is already parsed for you in response.body, so no need to re-parse.
frontend/simple/views/SignUp.vue
Outdated
| ] | ||
| }) | ||
| await namespace.register(this.name, user.toHash()) | ||
| await db.storeLogin(user.toHash(), multihash.toB58String(multihash.encode(new Buffer(this.password), 'blake2b'))) |
There was a problem hiding this comment.
OK, for now can you just directly store the password + add a TODO comment? That way I can hook in the crypto stuff later when it's ready, as simply storing the hash isn't secure (and this is also not how one would do it, i.e. it's missing the actual black2 stuff).
frontend/simple/views/SignUp.vue
Outdated
| forwardToLogin: function () { | ||
| Vue.events.$emit('login') | ||
| }, | ||
| checkName: async function () { |
There was a problem hiding this comment.
Let's have this called via a debounced manner each time the field changes. I.e. no more than calling it once per 2 seconds. If you search the vuejs guide for "debounce" you'll get several results, including this one:
Notice also the note at the bottom regarding throttle. Decide which provides a better experience and go for it. For importing debounce or throttle, see how it's done in state.js for debouncedSave.
| if (this.$route.query.next) { | ||
| setTimeout(() => { | ||
| this.$router.push({path: this.$route.query.next}) | ||
| }, 1000) |
There was a problem hiding this comment.
So that you can see the success message before transitioning
test/frontend.js
Outdated
| let randInt = (min, max) => Math.floor(Math.random() * (max - min)) + min | ||
| let username = `testuser${randInt(0, 10000000)}${randInt(10000000, 20000000)}` | ||
| it('Should register User', async function () { | ||
| this.timeout(10000) |
There was a problem hiding this comment.
Tests should never need such a large timeout, I would set this to 4000, max. If a test takes longer it's usually an indication of a really special circumstance or something wrong. Same for the timeouts below.
| /* There appears to be a bug in nightmare that causes the insert and type commands to enter old data into the field if the | ||
| insert or type commands or used more than once on the same field. This concatenates the old values to the new. | ||
| This occurs regardless of whether you clear the field or not. Until its fixed skip this validation test | ||
| */ |
There was a problem hiding this comment.
Hmm, did you try the Nightmare docs suggestion of inserting empty values to clear the field?
Both the .type and the .insert commands say they support it.
There was a problem hiding this comment.
Yes, this happens regardless of whether you clear the fields per their instructions.
test/frontend.js
Outdated
| await n.goto(page('signup')) | ||
| let badUsername = 't e s t' | ||
| let badEmail = `@fail` | ||
| // let badPassword = `789`// six is so afraid |
There was a problem hiding this comment.
Great job on making sure to test our error conditions! This greatly helps our coverage.
frontend/simple/js/database.js
Outdated
| } | ||
| export function retrieveLogin (identity: string): Promise<Object> { | ||
| return loginInfo.getItem(identity) | ||
| } |
There was a problem hiding this comment.
Is it possible to do this as part of the settings? The 'testUser' above there was meant to be a reminder to use that for all user-specific settings.
| } | ||
| }, | ||
| forwardToLogin: function () { | ||
| Vue.events.$emit('login') |
There was a problem hiding this comment.
Can you make it so that the username field is highlighted and ready to be typed in when the login thing is brought up?
frontend/simple/views/SignUp.vue
Outdated
| @@ -16,23 +16,27 @@ | |||
| <div class="box centered" style="max-width:400px"> | |||
| <h2 class="subtitle">Sign Up</h2> | |||
|
|
|||
| <a v-on:click="forwardToLogin" style="margin-left: 65%">Already a member?</a> | |||
There was a problem hiding this comment.
Can you change this to, "Have an account?"
| } | ||
| }, | ||
| logout: function () { | ||
| this.$store.commit('logout') |
There was a problem hiding this comment.
Clicking logout should take us to the home view (/simple/).
| this.password = null | ||
| this.errors.clear() | ||
| // https://github.com/oneuijs/You-Dont-Need-jQuery#css--style | ||
| this.$refs.modal.classList.toggle('is-active') |
There was a problem hiding this comment.
So the login modal should disappear if we click anywhere on the background or hit the 'esc' key.
…Debounce Name Check
No description provided.