Wiredcraft / test-fullstack

6 stars 43 forks source link

MaYi's work #4

Closed flyingant closed 8 years ago

flyingant commented 8 years ago

those pictures are the basic design for the project which help to create react component and this is NOT final design.

wwayne commented 8 years ago

@flyingant Hi, just wondering how you make use of less to set class? For example, in HomePage.js, I see you import styles from ../../less/home.less then you set it to <div className={styles.container}>, I don't know if it can works, could you tell me why you choose this way to load style? Thanks

wwayne commented 8 years ago

@flyingant okay, you can answer me later when you swim ashore... take care urself

flyingant commented 8 years ago

@wwayne Haha, Wuhan is safe now. Thanks for consideration!

For your question, with the loader style-loader and css-loader, webpack could compile those .less or .css files into a JS module and exports an object (we called css module) which contains all mappings with classnames defined in your .less or .css file. In short, it compiled the styles as a object and make it as a JS module.

the following are the reason why choose this way:

  1. Yep, no more global scope.
  2. no more conflicts, those classname will be compiled to a hash string
  3. explicit dependencies for react component
  4. easy to compose the style. you could even do this: .classA {color:red} .classB {composes: classA}
  5. avoid 'inline-style' in react component.
  6. no need to link external .css file.

You could also check the this link to find more about css moudle. https://github.com/css-modules/css-modules

wwayne commented 8 years ago

Yeah I know css-modules, just never thought about combining it with less, anyway, good to know that : )

Since css module focus on module, how do you think put the less file with js file together? for example: Build a new folder under app/components/ named LightingTalk, and put related js and less file into it

app/
 -- components/
     -- LightingTalk/
        -- index.js
        -- list.js
        -- item.js
        -- style.less

Just for discussion, you don't have to change your structure

Another question is for the state division, I see you only expose two states app and ui from reducer, for me, I would prefer to have another state talk for example, so that only lightingTalk component will Subscribe this state, maybe I'm wrong because I'm not very clear about the requirement. But do you have any reason that only have app and ui state?

flyingant commented 8 years ago

Yes! Fully agree with that to put the related file togethe or even we could pack those files into a npm package.

About the state division you are absolutly right. But for me I am just not willing to make it complex and also I have my own explation.

As we know, we would like to treat the component as Smart component (also called Stateful componentor Container componennt) and Dummy component( also called Stateless component or Presentational component) especially in redux development. In my opinion, only the Container component should Subscribe the related state and Presentational component should only accpet the state as Props from the container componennt's state. Then I through there's no need to divide the root state if my container components are not contain other nest container component.

all my container components are like:

Container component (app)
    -- Presentational component (app.foo)
    -- Presentational component (app.bar)

for the situation below, of course I should Subscribe state to the nest Container componennt

Container component (app)
    -- Container componennt (app.foo)
        -- Presentational component (app.foo.cat)
    -- Presentational component (app.bar)

But anyway I'm still agree to divide the root state into states because it gonna make the code clean and easy to understand. 💯

MaYi

wwayne commented 8 years ago

Okay cool : )

makara commented 8 years ago

Hi @flyingant you can continue hack this if you want, but I'm closing the PR.