Create JSS-styled prototype by lttb · Pull Request #1 · cssinjs/styled-jss

lttb · 2017-04-19T17:09:31Z

Implement base jss-styled realisation

TODO:

We increase amount of styles rendered to css in the head. so we should use just one global sheet?
Make is-native-prop as a separate package. Created is-react-prop

kof · 2017-04-19T17:13:21Z

+    "jss": "^6.5.0",
+    "jss-preset-default": "^1.3.1",
+    "react": "^15.4.2",
+    "react-jss": "^5.4.0",


both jss and react-jss are old, also if you have react-jss, you don't need jss and jss-preset-default

right, thanks!

kof · 2017-04-19T17:13:25Z

+    "recompose": "^0.23.1"
+  },
+  "devDependencies": {
+    "@lttb/eslint-config-default": "github:lttb/configs#js",


in case we are going to move this repo to the cssinjs org, we should use https://github.com/cssinjs/eslint-config-jss

kof · 2017-04-19T17:18:05Z

+
+    const elementStyles = { ...inheritStyles, ...styles }
+
+    const StyledElement = pure(


not sure shallowEqual should be implemented by default, does styled-components has it ?

if yes, instead of the library, we could use PureComponent from react, its some lines more but at the end way smaller build

I think it should, otherwise it may cause some effects with dom-elements like focus-lost on input in rerender

Please check what styled does out of curiosity

kof · 2017-04-19T17:20:44Z

+
+    const StyledElement = pure(
+      injectSheet({ [Tag]: elementStyles, ...baseStyles })(({ classes, children }) =>
+        <Tag className={classes[Tag]}>


I think styled supports className prop, so if user passes a className it should not override, it should be added

kof · 2017-04-19T17:21:24Z

+    const elementStyles = { ...inheritStyles, ...styles }
+
+    const StyledElement = pure(
+      injectSheet({ [Tag]: elementStyles, ...baseStyles })(({ classes, children }) =>


I am not sure its a good idea to have a separate style tag for each styled element, that would be a lot of tags.

kof · 2017-04-19T19:00:33Z

+  },
+  "homepage": "https://github.com/lttb/jss-styled#readme",
+  "dependencies": {
+    "react": "^15.5.4",


react should be in peerDependencies https://github.com/styled-components/styled-components/blob/master/package.json#L118

kof · 2017-04-19T19:05:45Z

+    const elementStyles = { ...inheritStyles, ...styles }
+
+    const Primitive = ({ classes, children, className }: PrimitiveProps) =>
+      <Tag className={classes[Tag].concat(className ? ` ${className}` : '')}>


how is it possible that you use Tag as a component and as a class name at the same time?

kof · 2017-04-19T19:20:07Z

+      styles?: Object
+    } = typeof Element === 'string' ? { Tag: Element } : Element
+
+    const elementStyles = { ...inheritStyles, ...styles }


In this case we increase amount of styles rendered to css, we should think how to leverage jss-compose for this.

kof · 2017-04-19T19:21:07Z

+
+export const styled = prepareStyled()
+
+export const setStyledCreator = (styledFunction: Function = styled) =>


name setStyledCreator is bad, this function doesn't set anything, its a get or a create

createStyledCreator is a better name

kof · 2017-04-19T19:31:55Z

+}
+
+
+export const prepareStyled = (injectSheet?: Function = injectSheetDefault) =>


createStyled is a better name

kof · 2017-04-19T19:34:32Z

+    return Object.assign(StyledPrimitive, { Tag, styles: elementStyles })
+  }
+
+export const styled = prepareStyled()


suggestion for the naming conflict fix

const defaultStyled = createStyled() export {defaultStyled as styled} export const createStyledCreator = (styled: Function = defaultStyled) =>

kof · 2017-04-19T19:35:44Z

+
+export const prepareStyled = (injectSheet?: Function = injectSheetDefault) =>
+  (
+    Element: string | StyledElementType,


I am using in such cases this convention: TagNameOrStyledElement

later on you can do the delegation and name the variable differently.

kof · 2017-04-19T19:36:53Z

+
+export default (styled: Function) => {
+  const App = styled('div', {
+    margin: '50px',


please rely in all examples on jss-preset-default, in this case it allows you to use numbers

kof · 2017-04-19T19:37:20Z

+  })
+
+  const Button = styled('button', {
+    margin: ({ margin = 0 }) => `${margin}px`,


same here, you can use numbers in the latest jss version

kof · 2017-04-19T19:37:33Z

+
+      <Section data-name="content">
+        <Button>primitive test</Button>
+        <Button _margin={10}>dynamic primitive test</Button>


forgot to update?

kof · 2017-04-20T22:25:14Z

@@ -0,0 +1,133 @@
+import React, {PureComponent} from 'react'
+
+import {create as createJSS, getDynamicStyles} from 'jss'


I use strict camel case: createJss, it looks strange sometimes but its a better way.

kof · 2017-04-20T22:32:37Z

+
+const JSS = createJSS(preset())
+
+type StyledElementAttrsT = { Tag: string, styles: Object }


do types conflict with vars? Asking because of "T" suffix.

yes, they can't have the same names

kof · 2017-04-20T23:52:00Z

+import filterProps from './utils/filter-props'
+
+
+const JSS = createJSS(preset())


I use jss small for the Jss instance.

yep, fix it here 7eb58c1#diff-73642d587163f2eb4d72f0fa6ebfba02R9

kof · 2017-04-20T23:55:31Z

+  let sheet
+  let dynamicSheet
+
+  let currentId = 0


its more like a counter I think

kof · 2017-04-20T23:59:14Z

+import injectSheet from 'react-jss'
+
+// Base styles, like a regular jss object.
+const styled = Styled({


I think this should be a second separate example, because user will think he needs to do that.

I would try to make clear that the API is mostly compatible with styled

And this feature is on top of it.

kof · 2017-04-21T00:00:54Z

+
+const NormalButton = styled('button', {
+  composes: '$baseButton',
+  border: '1px solid grey'


Lets leverage jss-default-unit? border: [1, 'solid', 'grey']

Implement injectStyled

lttb added 2 commits April 19, 2017 20:11

Create JSS-styled base

6e432fe

Create JSS-styled prototype

6a80dd8

lttb force-pushed the feature/prototype branch from cab7cef to 6a80dd8 Compare April 19, 2017 17:11

kof reviewed Apr 19, 2017

View reviewed changes

lttb added 2 commits April 19, 2017 21:16

Remove pure component and support custom className

89a3dd7

Upgrade deps and use eslint-config-jss

e960f4b

kof reviewed Apr 19, 2017

View reviewed changes

lttb added 3 commits April 20, 2017 01:34

Update flow-typed

4b9b6ad

Implement property validation with is-native-prop

3fe85ad

Use is-react-prop

f756cef

lttb force-pushed the feature/prototype branch 2 times, most recently from 4edbac4 to 6e1b3fe Compare April 20, 2017 15:58

Implement common style sheets

f8a7e59

lttb force-pushed the feature/prototype branch from 6e1b3fe to f8a7e59 Compare April 20, 2017 16:01

Use common dynamic stylesheet

682aee9

lttb force-pushed the feature/prototype branch from f036f28 to 682aee9 Compare April 20, 2017 17:04

kof reviewed Apr 20, 2017

View reviewed changes

Comment thread src/index.jsx Outdated

let sheet

let dynamicSheet

let currentId = 0

Copy link
Copy Markdown

Member

kof Apr 20, 2017

Choose a reason for hiding this comment

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

its more like a counter I think

Fix naming

0e45a63

lttb force-pushed the feature/prototype branch from 7eb58c1 to 0e45a63 Compare April 20, 2017 23:56

kof reviewed Apr 20, 2017

View reviewed changes

Use updateRule from jss

e87dc0e

kof reviewed Apr 21, 2017

View reviewed changes

kof merged commit 7e7a507 into master Apr 21, 2017

lttb deleted the feature/prototype branch April 21, 2017 10:43

lttb mentioned this pull request Apr 23, 2017

Code Style: express acronyms in capital space #8

Closed

lttb added a commit that referenced this pull request Apr 28, 2017

Merge pull request #1 from lttb/feature/inject-styled

9bdb073

Implement injectStyled


		const elementStyles = { ...inheritStyles, ...styles }

		const StyledElement = pure(


		export const styled = prepareStyled()

		export const setStyledCreator = (styledFunction: Function = styled) =>

		}


		export const prepareStyled = (injectSheet?: Function = injectSheetDefault) =>

		@@ -0,0 +1,133 @@
		import React, {PureComponent} from 'react'

		import {create as createJSS, getDynamicStyles} from 'jss'


		const JSS = createJSS(preset())

		type StyledElementAttrsT = { Tag: string, styles: Object }

		import filterProps from './utils/filter-props'


		const JSS = createJSS(preset())

Conversation

lttb commented Apr 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lttb commented Apr 19, 2017 •

edited

Loading