Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed: localStorage not defined in server side #91

Merged
merged 4 commits into from
Jan 7, 2017
Merged

Fixed: localStorage not defined in server side #91

merged 4 commits into from
Jan 7, 2017

Conversation

comus
Copy link
Contributor

@comus comus commented Jan 6, 2017 •
edited
Loading

see: #89

  • check onChange exists before call it
  • noValidate
  • check localStorage, check backwards on client side, choose username or usernameOrEmail corrently

@comus
Copy link
Contributor Author

comus commented Jan 7, 2017

in Field, allow triggerUpdate for this.input.value = '' (empty) is necessary.

signIn function need this.state.usernameOrEmail !== null or this.state.username !== null
if not triggerUpdate for value='', it means i just press login button on webpage.
the loginform will not alert usernameRequired,
because at that time this.state.usernameOrEmail and this.state.username are not exist

so, allow triggerUpdate for value = '', it helps to set default state:
componentDidMount -> triggerUpdate
then triggerUpdate (this.input.value = '') -> onChange
then onChange -> handleChange
then handleChange -> setState (the result is this.state.usernameOrEmail = '' or this.state.username = '')

@timbrandin timbrandin changed the title fixed something for ssr Fixed: localStorage not defined in server side Jan 7, 2017
@timbrandin timbrandin self-requested a review January 7, 2017 10:13
timbrandin
timbrandin reviewed Jan 7, 2017
View reviewed changes
imports/helpers.js
if ( username ) {
return true;
} else {
showMessage(T9n.get("error.usernameRequired"), 'warning', false, 'username');
const fieldName = (passwordSignupFields() === 'USERNAME_ONLY' || formState === STATES.SIGN_UP) ? 'username' : 'usernameOrEmail';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't think of this! Great!

@timbrandin timbrandin merged commit 7edc744 into studiointeract:master Jan 7, 2017
@timbrandin
Copy link
Member

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timbrandin timbrandin timbrandin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@comus @timbrandin