LBRYCommunity/lbry-android
1
0
Fork 0
Code Issues 155 Pull requests 6 Projects Releases 84 Packages Wiki Activity

complete authentication flow and email verification link implementation #232

Merged
akinwale merged 9 commits from authentication-flow into master 2018-08-16 11:48:35 +02:00
Conversation 0 Commits 9 Files changed 16 +490 -78
akinwale commented 2018-08-15 17:59:00 +02:00 (Migrated from github.com)
Copy link
No description provided.
neb-b (Migrated from github.com) reviewed 2018-08-15 17:59:00 +02:00
skhameneh (Migrated from github.com) approved these changes 2018-08-16 10:34:31 +02:00
skhameneh (Migrated from github.com) left a comment
Copy link

Mostly minor comments aside from some things about lifecycle methods

Mostly minor comments aside from some things about lifecycle methods
app/src/component/AppNavigator.js
@ -144,7 +161,7 @@ class AppWithNavigationState extends React.Component {
}
skhameneh (Migrated from github.com) commented 2018-08-16 10:19:21 +02:00
Copy link

You can define this without a constructor
https://hackernoon.com/the-constructor-is-dead-long-live-the-constructor-c10871bea599

You can define this without a constructor https://hackernoon.com/the-constructor-is-dead-long-live-the-constructor-c10871bea599
app/src/constants.js
@ -1,5 +1,10 @@
const Constants = {
SETTING_ALPHA_UNDERSTANDS_RISKS: "ALPHA_UNDERSTANDS_RISKS"
KEY_FIRST_RUN_EMAIL: "firstRunEmail",
KEY_SHOULD_VERIFY_EMAIL: "shouldVerifyEmail",
skhameneh (Migrated from github.com) commented 2018-08-16 10:20:58 +02:00
Copy link

Why is this value camel case unlike the others?

Why is this value camel case unlike the others?
app/src/page/firstRun/internal/email-collect-page.js
skhameneh (Migrated from github.com) commented 2018-08-16 10:22:29 +02:00
Copy link

No aliases (or resolves) for paths?
*Remove the long relative paths

No aliases (or resolves) for paths? *Remove the long relative paths
skhameneh (Migrated from github.com) commented 2018-08-16 10:25:25 +02:00
Copy link

Don’t need a full constructor (see comment above)

Don’t need a full constructor (see comment above)
skhameneh (Migrated from github.com) commented 2018-08-16 10:27:30 +02:00
Copy link

This is a legacy lifecycle method, should use getDerivedStateFromProps

This is a legacy lifecycle method, should use `getDerivedStateFromProps`
app/src/page/firstRun/view.js
@ -77,0 +100,4 @@
// validate the email
AsyncStorage.getItem(Constants.KEY_FIRST_RUN_EMAIL).then(email => {
if (!email || email.trim().length === 0 || email.indexOf('@') === -1) {
return notify({
skhameneh (Migrated from github.com) commented 2018-08-16 10:29:39 +02:00
Copy link

Legacy method, see comment above

Legacy method, see comment above
app/src/page/firstRun/view.js
skhameneh (Migrated from github.com) commented 2018-08-16 10:31:43 +02:00
Copy link

Nice and simple 👍
There’s an RFC conforming regex I know of, I’ll try to pull it up tomorrow

Nice and simple 👍 There’s an RFC conforming regex I know of, I’ll try to pull it up tomorrow
app/src/page/splash/view.js
@ -55,0 +91,4 @@
} else {
notify({
message: 'Invalid Verification URI',
displayType: ['toast'],
skhameneh (Migrated from github.com) commented 2018-08-16 10:33:15 +02:00
Copy link

Legacy lifecycle method, see other comments about this

Legacy lifecycle method, see other comments about this
akinwale (Migrated from github.com) reviewed 2018-08-16 10:51:26 +02:00
app/src/constants.js
@ -1,5 +1,10 @@
const Constants = {
SETTING_ALPHA_UNDERSTANDS_RISKS: "ALPHA_UNDERSTANDS_RISKS"
KEY_FIRST_RUN_EMAIL: "firstRunEmail",
KEY_SHOULD_VERIFY_EMAIL: "shouldVerifyEmail",
akinwale (Migrated from github.com) commented 2018-08-16 10:51:26 +02:00
Copy link

The value for the SETTING_* key should actually be camel case.

The value for the `SETTING_*` key should actually be camel case.
Sign in to join this conversation.
Reviewers
No reviewers
neb-b
skhameneh
Labels
Clear labels   
android: closed alpha

   
android: open beta

   
app-parity

   
area: devops

   
area: discovery

   
area: docs

   
area: livestream

   
area: proposal

Work is part of a proposal

  
consider soon

Discuss this issue at the next planning meeting, then remove this label

  
creator

   
Epic

   
good first issue

   
hacktoberfest

Welcome to Hacktoberfest

  
help wanted

   
icebox

Long-term storage

  
Invalid

   
level: 1

No knowledge of the existing code required

  
level: 2

Some knowledge of the existing code is recommended

  
level: 3

Significant knowledge of the existing code is recommended

  
level: 4

Intimate knowledge of the existing code is recommended

  
needs: exploration

Solution unclear, needs research

  
needs: grooming

Issue needs to be groomed before work can start

  
needs: priority

Priority level needs to be defined

  
needs: repro

Issue needs step-by-step instructions on how to reproduce in latest code

  
needs: tech design

Needs technical design signoff before implementation can begin

  
on hold

Temporarily paused

  
priority: blocker

Issue is blocking release, do ASAP

  
priority: high

Work needs to be moved into sprint ASAP

  
priority: low

Work should be done but can stay in the backlog for now

  
priority: medium

Work needs to be done within 2-3 sprints

  
product review

   
resilience

   
Tom's Wishlist

   
type: bug

Existing functionality is wrong or broken

  
type: discussion

A conversation. No specific changes requested

  
type: improvement

Existing (or partially existing) functionality needs to be changed

  
type: new feature

New functionality that does not exist yet

  
type: refactor

Minimal user-visible changes, but significant internal work

  
type: task

Either work that's not related to the code, or a small chore that does not fit into other categories

  
type: testing

Solution needs additional user testing

  
unplanned

Work that was not planned into the spirnt, took priority over planned work

No labels
android: closed alpha
android: open beta
app-parity
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
creator
Epic
good first issue
hacktoberfest
help wanted
icebox
Invalid
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
priority: blocker
priority: high
priority: low
priority: medium
product review
resilience
Tom's Wishlist
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-android#232
No description provided.
Delete branch "authentication-flow"

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?