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.

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

Proposal: React Library Spec #58

Closed
amorey opened this issue Dec 3, 2015 · 21 comments
Closed

Proposal: React Library Spec #58

amorey opened this issue Dec 3, 2015 · 21 comments
Labels

Comments

@amorey
Copy link
Member

amorey commented Dec 3, 2015

Hi Everyone,

We're planning an upgrade to the MUI React library to make it production-ready. Please let us know what you think of this spec:

Components

Appbar

<Appbar/>

Buttons

<Button/>
  * {String} color=default|primary|danger|accent
  * {String} variant=default|flat|raised|fab
  * {String} size=default|small|large
  * {Boolean} isDisabled=false|true

Caret

<Caret/>

Container

<Container/>
  * {Boolean} isFluid=false|true

Dividers

<Divider/>

Dropdowns

<Dropdown/>
  * {String} color=default|primary|danger|accent
  * {String} variant=default|flat|raised|fab
  * {String} size=default|small|large
  * {String} label
  * {String} alignMenu=left|right
  * {Function} onClick
  * {Boolean} isDisabled

<DropdownItem/>
  * {String} link
  * {Function} onClick

Forms

<Form/>
  * {Boolean} isInline=false|true

<TextInput/>
  * {String} type=text|email|password
  * {String} hint
  * {String} label
  * {Boolean} isLabelFloating=false|true
  * {Boolean} isRequired=false|true

<TextareaInput/>
  * {Integer} rows
  * {String} hint
  * {String} label
  * {Boolean} isLabelFloating=false|true
  * {Boolean} isRequired=false|true

<Checkbox/>
  * {String} value
  * {String} label
  * {Boolean} isChecked=false|true
  * {Boolean} isDisabled=false|true

<Radio/>
  * {String} name
  * {String} value
  * {Boolean} isChecked=false|true
  * {Boolean} isDisabled=false|true

<Select/>
  * {String} name
  * {Boolean} isAutofocus=false|true
  * {Boolean} isDisabled=false|true
  * {Boolean} isMultiple=false|true
  * {Boolean} isRequired=false|true
  * {Boolean} useDefault=false|true
  * {Function} onChange

<SelectItem/>
  * {String} value
  * {String} label

Grid

<Row/>

<Col/>
  * {Integer} xs
  * {Integer} xs-offset
  * {Integer} sm
  * {Integer} sm-offset
  * {Integer} md
  * {Integer} md-offset
  * {Integer} lg
  * {Integer} lg-offset

Panel

<Panel/>

Tabs

<Tabs/>
  * {Integer} initialSelectedIndex=0
  * {Boolean} isJustified=false|true
  * {Function} onChange

<Tab/>
  * {String} label
  * {String} value
  * {Function} onActive

CSS Helpers

<!-- animation -->
<div className="mui--no-transition"></div>

<!-- alignment -->
<div className="mui--text-left"></div>
<div className="mui--text-right"></div>
<div className="mui--text-center"></div>
<div className="mui--text-justify"></div>
<div className="mui--text-nowrap"></div>
<div className="mui--align-baseline"></div>
<div className="mui--align-top"></div>
<div className="mui--align-middle"></div>
<div className="mui--align-bottom"></div>

<!-- depth helpers -->
<div className="mui--z1"></div>
<div className="mui--z2"></div>
<div className="mui--z3"></div>
<div className="mui--z4"></div>
<div className="mui--z5"></div>

<!-- float helpers -->
<div className="mui--clearfix"></div>
<div className="mui--pull-right"></div>
<div className="mui--pull-left"></div>

<!-- toggle helpers -->
<div className="mui--hide"></div>
<div className="mui--show"></div>
<div className="mui--invisible"></div>
<div className="mui--overflow-hidden"></div>

<!-- responsive utilities -->
<div className="mui--visible-xs-block"></div>
<div className="mui--visible-xs-inline"></div>
<div className="mui--visible-xs-inline-block"></div>
<div className="mui--visible-sm-block"></div>
<div className="mui--visible-sm-inline"></div>
<div className="mui--visible-sm-inline-block"></div>
<div className="mui--visible-md-block"></div>
<div className="mui--visible-md-inline"></div>
<div className="mui--visible-md-inline-block"></div>
<div className="mui--visible-lg-block"></div>
<div className="mui--visible-lg-inline"></div>
<div className="mui--visible-lg-inline-block"></div>
<div className="mui--hidden-xs"></div>
<div className="mui--hidden-sm"></div>
<div className="mui--hidden-md"></div>
<div className="mui--hidden-lg"></div>

<!-- typograpy -->
<div className="mui--text-display4"></div>
<div className="mui--text-display3"></div>
<div className="mui--text-display2"></div>
<div className="mui--text-display1"></div>
<div className="mui--text-headline"></div>
<div className="mui--text-title"></div>
<div className="mui--text-subhead"></div>
<div className="mui--text-body2">Body2</div>
<div className="mui--text-body1">Body1</div>
<div className="mui--text-caption">Caption</div>
<div className="mui--text-menu">Menu</div>
<div className="mui--text-button">Button</div>

<!-- text color -->
<div className="mui--text-dark-primary"></div>
<div className="mui--text-dark-secondary"></div>
<div className="mui--text-dark-hint"></div>

<div className="mui--text-light-primary"></div>
<div className="mui--text-light-secondary"></div>
<div className="mui--text-light-hint"></div>

<div className="mui--text-accent"></div>
<div className="mui--text-accent-fallback"></div>

<div className="mui--text-danger"></div>

<!-- user select -->
<div className="mui--no-user-select"></div>

<!-- appbar dimension helpers -->
<div className="mui--appbar-height"></div>
<div className="mui--appbar-min-height"></div>
<div className="mui--appbar-line-height"></div>

<!-- list helpers -->
<ul className="mui-list--unstyled"></ul>
<ul className="mui-list--inline"></ul>

JavaScript API

Overlay

<script>
  mui.overlay('on'[, <options>[, <childEl>]]);
  mui.overlay('off');
</script>
@amorey amorey added the question label Dec 3, 2015
This was referenced Dec 3, 2015
@defrex
Copy link
Contributor

defrex commented Dec 3, 2015

This looks great, and is very exciting. I have a couple points of feedback.

On the MUIButton and MUIDropdown components you're using the attribute style. This is reserved in React for inline CSS styles so you'll need to change it to something else.

The col- prefix for the MUICol attributes seems redundant <MUICol sm={3}/> would be much cleaner then <MUICol col-sm={3}/>.

Prefixing everything with MUI seems like unnecessary clutter. I don't know anyone working on React codebases who isn't using some kind of module system. In that kind of no-globals environment this kind of namespacing is just noise.

For example:

import { Button } from 'muicss/react';

const MyComponent = ()=> <Button>+1</Button>;

That is nice and clean. In the off chance I have a conflict with the name Button, I can deal with that myself.

import { Button } from 'react-bootstrap';
import { Button as MUIButton } from 'muicss/react';

const MyComponent = ()=> <MUIButton>+1</MUIButton>;

Or even

import { Button } from 'react-bootstrap';
import MUI from 'muicss/react';

const MyComponent = ()=> <MUI.Button>+1</MUI.Button>;

This kind of approach is much more consistent with the best practices in the React ecosystem.

Thanks for all the work here! I think there is a real opportunity for a Material UI React component set. material-ui is nice and all, but it uses exclusively inline styles, which turns off a lot of people (myself included). MUI is a nice clean CSS framework and a suit of components that simply uses those classes will be much nicer to use and customize.

@amorey
Copy link
Member Author

amorey commented Dec 4, 2015

Thanks for your excellent feedback! I've incorporated your suggestions into the spec (see above). Do you have any recommendations for an attribute name to replace "style"?

@defrex
Copy link
Contributor

defrex commented Dec 4, 2015

I'm not sure. variant maybe? Two hard problems, amirite?

@amorey
Copy link
Member Author

amorey commented Dec 4, 2015

Good thing we don't have to worry about cache invalidation.

Do you have a preference?

  • variant
  • type
  • btnType
  • something else?

The MD spec refers to them as "Button Types":
https://www.google.com/design/spec/components/buttons.html#buttons-button-types

@defrex
Copy link
Contributor

defrex commented Dec 4, 2015

type can be hairy in some contexts since it's reserved in JS (though it will still work fine in JSX). btnType seems reasonable, if a little inelegant.

@amorey
Copy link
Member Author

amorey commented Dec 4, 2015

Agreed... I'm leaning towards 'variant'.

On Dec 4, 2015, at 5:37 PM, Aron Jones notifications@github.com wrote:

type can be harry in some contexts since it's reserved in JS (though it will still work fine in JSX). btnType seems reasonable, if a little inelegant.


Reply to this email directly or view it on GitHub.

@amorey
Copy link
Member Author

amorey commented Dec 21, 2015

@defrex I have a version of the MUI React library that implements the proposed spec. I want to spend time optimizing the code and writing more unit tests but it'd be useful to get your thoughts on the code beforehand:
https://github.com/muicss/mui/tree/react/src/react

Note that it isn't being exposed via 'muicss/react' yet. Also, I haven't updated the Button module to ES6 yet because I ran into issues with React ES6 mixin support and decided to work on it later.

@ghost
Copy link

ghost commented Jan 4, 2016

@amorey This is looking pretty awesome!
Do you think you could expose it as muicss/react-next or something similar?
Or is there another way that these new components can be beta tested?

@ghost
Copy link

ghost commented Jan 4, 2016

I went ahead and built the react branch and am running the components through the paces in a sample application.

A couple of things that I've come across so far:

  1. React components aren't exported in a format consumable via webpack
  2. className and style props get dropped instead of merging into the underlying component's props

I'm happy to create a pull request for either or both of these if you'd be interested.

@amorey
Copy link
Member Author

amorey commented Jan 5, 2016

@danmartinez101 Great! Happy to hear you like the spec.

What problems did you run into with webpack? Was it an issue with ES6 or JSX syntax? I've been thinking about the best way to expose MUI via an NPM package and so far I've been playing around with using a build step to create a pkg from source. In the latest react branch version you can build the pkg with gulp:

$ ./node_modules/.bin/gulp build-pkg

Install the package using the local repo:

$ npm install /path/to/mui/pkg

And then access the exposed modules in nodejs:

> var overlay = require('muicss').overlay;
> var Button = require('muicss/react').Button;

I'm just starting to work on this so let me know if you have any recommendations on how to expose MUI via NPM and make it work with webpack.

With regards to the className and style props issue that'd be great if you could create a pull request!

@amorey
Copy link
Member Author

amorey commented Jan 7, 2016

@danmartinez101 Thanks for the react peerDependency recommendation in the react-packages branch. I made the change and pushed to github.

I've been playing around with your "monorepo" idea in that branch and so far I think it's looking pretty good. If you want to try to split up the work let me know. I think we can use this gulp pattern to cleanly divide up the build tasks: https://github.com/muicss/mui/blob/react-packages/gulpfile.js#L27-L30.

@ghost
Copy link

ghost commented Jan 9, 2016

That package structure is looking pretty clean :) 👍

I'll be free a bit next week, so I'll ping you on gitter and see if there is anything I can help out with.

@FoxxMD
Copy link

FoxxMD commented Jan 13, 2016

+1 awesome work guys! really looking forward to trying this out when it's ready.

@amorey
Copy link
Member Author

amorey commented Jan 17, 2016

@defrex @danmartinez101 @FoxxMD Thanks for your patience!

We now have a version of MUI React that implements the proposed spec above:
https://github.com/muicss/mui/tree/react

Based on @danmartinez101's recommendation I created a "monorepo" structure and split out the CDN/NPM/Meteor packages into separate directories. There's still work to do with Bowser and Meteor but here's the NPM package including the README:
https://github.com/muicss/mui/tree/react/packages/npm

If you have some time, please take a look at the README to see if you have any suggestions on how to improve the spec or the README itself. Once this is ready we can add more examples to https://www.muicss.com and then launch!

One more question - what is your preferred way to include CSS in React apps? Should we add mui.css and mui.min.css to the NPM package, leave it in the Bower package or leave it to the developer to decide how to include the CSS static file?

@defrex
Copy link
Contributor

defrex commented Jan 18, 2016

This looks great! Thanks for all the hard work.

Regarding CSS, I'd love to see both the CSS and the SASS source included in the NPM package. When using webpack you can easily import CSS from NPM deps, and if the user is using SASS, the source can be imported directly to enable custom builds.

@FoxxMD
Copy link

FoxxMD commented Jan 18, 2016

I agree with @defrex on including both css and sass in source. The README looks good! The links to the docs don't work but I'm sure that still a work in progress :)

@amorey
Copy link
Member Author

amorey commented Jan 18, 2016

@defrex @FoxxMD Thanks for the feedback! The links are a work in progress...

Can you point to an example of how to include SASS source files in an NPM package with webpack? Usually I use bower to import source files.

@FoxxMD
Copy link

FoxxMD commented Jan 18, 2016

I don't think there is anything special you need to do (but @defrex please correct me if I'm wrong). Including the sass folder along the lib folder for the npm package (or inside lib) would be enough I think. Then, instead of requiring the distributable css we can require mui.sass

@defrex
Copy link
Contributor

defrex commented Jan 18, 2016

Agreed. Doesn't matter much where it lives in the structure, as long as it's somewhere.

@amorey
Copy link
Member Author

amorey commented Jan 20, 2016

@danmartinez101 @defrex @FoxxMD The above spec has been implemented in v0.3.0-rc1 along with CSS and SASS files:
https://www.npmjs.com/package/muicss

The documentation on the main website is coming soon. Let me know what you think of the new version!

@amorey
Copy link
Member Author

amorey commented Jan 24, 2016

@danmartinez101 @defrex @FoxxMD Thanks again for your feedback. I'm closing this thread in favor of the new React Library Release Candidate Thread: #69

See you in the new thread!

@amorey amorey closed this as completed Jan 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants