llorien / crm

WordPress plugin installed on brithon.com to support managing customer relationship
0 stars 0 forks source link

Issue 19 #20

Closed excelle08 closed 9 years ago

excelle08 commented 9 years ago

User registration function

xueruini commented 9 years ago

will check it carefully.

excelle08 commented 9 years ago

Now the refactored code can run successfully.

However I don't understand why component.state is unavaliable. I switched to component.props.value to store tha value of text box.

excelle08 commented 9 years ago

OK I get that it's not a proper practice to use state.

xueruini commented 9 years ago

We use props and setProps in top level.

excelle08 commented 9 years ago

I'm just trying to take a deep insight into the new workflow and refactor the code. I had thought that it's not that necessary to use Immutable for the registration module, but not the case under the current class system.

xueruini commented 9 years ago

I think we can follow the airbnb style: https://github.com/airbnb/javascript

xueruini commented 9 years ago

@excelle08 We have a new architecture which is demonstrated in module todomvc. Please refer to it to refactor your code. Here are some key points.

Wei and I had a conf call yesterday talking about these updates.

xueruini commented 9 years ago

:shipit:

xueruini commented 9 years ago

Agree, let's begin with airbnb's and refine it later.

Airbnb provides linter configs: https://github.com/airbnb/javascript/tree/master/linters

I also found that it's convention is similar to the default settings of esformatter. So, we can use it's linter configs in sublime, while user esformatter to format.

excelle08 commented 9 years ago

ESFormatter cannot format some of the JS file. It says Error (1):Unable to format. Required plugins are installed and .eslintrc has been put under home directory.

BTW, are we really going to adapt the standard of using two spaces for indentation? It may cause mass code refactoring and what's more, it looks inelegant, not having enough space for readability.

xueruini commented 9 years ago

Though I used to 4 spaces personally, I prefer following the airbnb rule entirely for our project.

@excelle08 I am afraid you does not setup esformatter properly. It works.

.eslintrc needs sublime-linter and sublimelinter-contrib-eslint in Sublime Text 3 only.

In order to use tidyphp, we have to stick to Sublime Text 2. Then use .jshintrc instead.

excelle08 commented 9 years ago

@lukehl OK I will format the code with the guide. @xueruini Actually I managed to install TidyPHP on Sublime Text 3, but ESFormatter cannot format some of the file on both Sublime Text 2 and 3. I followed your instruction using npm install eslint babel-eslint eslint-plugin-react -g to install required plugins.

.jshintrc is also put on the home directory.

xueruini commented 9 years ago

I followed your instruction using npm install eslint babel-eslint eslint-plugin-react -g to install required plugins.

You have to also install sublimelinter and sublime-contrib-eslint for sublime text. This plugin will hint potential breaking rules. To use .jshintrc, sublime-jshint-gutter is required.

xueruini commented 9 years ago

Actually I managed to install TidyPHP on Sublime Text 3

How to you get it installed? Does it work properly?

excelle08 commented 9 years ago

@xueruini By adding repository https://github.com/welovewordpress to the package control and then install. Then go to ~/Library/Application Support/Sublime Text 3/Package and clone the repo https://github.com/welovewordpress/SublimePhpTidy.

excelle08 commented 9 years ago

They are reformatted now. Finally I found that it's the comment that crashed ESFormatter. @xueruini @lukehl

excelle08 commented 9 years ago

OK

xueruini commented 9 years ago

By adding repository https://github.com/welovewordpress to the package control and then install. Then go to ~/Library/Application Support/Sublime Text 3/Package and clone the repo https://github.com/welovewordpress/SublimePhpTidy.

good!