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

#212 - remove theming #225

Merged
merged 3 commits into from
Aug 3, 2017
Merged

#212 - remove theming #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32099c1
#212 - remove theming
Aug 2, 2017
aaff0f1
Merge branch 'master' into remove-theming
Aug 3, 2017
fed2a14
Apply all classes for components as selectors
emmatown Aug 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/react/constants.js
View file
Open in desktop

This file was deleted.

135 changes: 50 additions & 85 deletions src/react/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Component, createElement as h } from 'react'
import PropTypes from 'prop-types'
import { createElement as h } from 'react'
import { css } from '../index'
import { map, omit, reduce, assign } from '../utils'
import { CHANNEL } from './constants'

export {
flush,
Expand All @@ -25,106 +23,73 @@ export default function (tag, cls, objs, vars = [], content) {

const componentTag = tag.displayName || tag.name || 'Component'

class Styled extends Component {
state = {
theme: {}
}

componentWillMount () {
if (this.context[CHANNEL]) {
this.setState({ theme: this.context[CHANNEL].getState() })
}
}

componentDidMount () {
if (this.context[CHANNEL]) {
this.unsubscribe = this.context[CHANNEL].subscribe(this.setTheme)
}
}

componentWillUnmount () {
if (typeof this.unsubscribe === 'function') {
this.unsubscribe()
}
}

render () {
const { props, state, context } = this
const mergedProps = assign({}, props, {
theme: state.theme
})

const getValue = v => {
if (v && typeof v === 'function') {
if (v.__emotion_class) {
return `& .${v.__emotion_class}`
}
return v(mergedProps, context)
function Styled (props, context) {
const getValue = v => {
if (v && typeof v === 'function') {
if (v.__emotion_class) {
return `& .${v.__emotion_class}`
}

return v
return v(props, context)
}
let localTag = tag

let finalObjs = []

if (tag.__emotion_spec) {
push(
finalObjs,
reduce(
tag.__emotion_spec,
(accum, spec) => {
push(accum, spec.objs)
if (spec.content) {
push(accum, spec.content.apply(null, map(spec.vars, getValue)))
}
return accum
},
[]
)
return v
}
let localTag = tag

let finalObjs = []

if (tag.__emotion_spec) {
push(
finalObjs,
reduce(
tag.__emotion_spec,
(accum, spec) => {
push(accum, spec.objs)
if (spec.content) {
push(accum, spec.content.apply(null, map(spec.vars, getValue)))
}
push(accum, [spec.cls])
return accum
},
[]
)
localTag = tag.__emotion_spec[0].tag
}

push(finalObjs, objs)

push(finalObjs, [cls])
)
localTag = tag.__emotion_spec[0].tag
}

if (content) {
push(finalObjs, content.apply(null, map(vars, getValue)))
}
push(finalObjs, objs)

if (mergedProps.className) {
push(finalObjs, mergedProps.className.split(' '))
}
push(finalObjs, [cls])

const className = css(map(finalObjs, getValue))
if (content) {
push(finalObjs, content.apply(null, map(vars, getValue)))
}

return h(
localTag,
omit(
assign({}, mergedProps, {
ref: mergedProps.innerRef,
className
}),
['innerRef', 'theme']
)
)
if (props.className) {
push(finalObjs, props.className.split(' '))
}

setTheme = theme => this.setState({ theme })
}
const className = css(map(finalObjs, getValue))

Styled.contextTypes = {
[CHANNEL]: PropTypes.object
return h(
localTag,
omit(
assign({}, props, {
ref: props.innerRef,
className
}),
['innerRef', 'theme']
)
)
}

Styled.displayName = `styled(${componentTag})`
const spec = {
vars,
content,
objs,
tag
tag,
cls
}
Styled.__emotion_spec = tag.__emotion_spec
? tag.__emotion_spec.concat(spec)
Expand Down
6 changes: 0 additions & 6 deletions src/react/theming.js
View file
Open in desktop

This file was deleted.

71 changes: 44 additions & 27 deletions test/__snapshots__/react.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,21 @@ exports[`styled change theme 1`] = `
}
}
>
<styled(Component)>
<div
className="glamor-0 glamor-1"
<WithTheme(styled(Component))>
<styled(Component)
theme={
Object {
"primary": "green",
}
}
>
this will be green then pink
</div>
</styled(Component)>
<div
className="glamor-0 glamor-1"
>
this will be green then pink
</div>
</styled(Component)>
</WithTheme(styled(Component))>
</ThemeProvider>
</TestComponent>
`;
Expand All @@ -88,13 +96,21 @@ exports[`styled change theme 2`] = `
}
}
>
<styled(Component)>
<div
className="glamor-0 glamor-1"
<WithTheme(styled(Component))>
<styled(Component)
theme={
Object {
"primary": "pink",
}
}
>
this will be green then pink
</div>
</styled(Component)>
<div
className="glamor-0 glamor-1"
>
this will be green then pink
</div>
</styled(Component)>
</WithTheme(styled(Component))>
</ThemeProvider>
</TestComponent>
`;
Expand Down Expand Up @@ -177,13 +193,13 @@ exports[`styled component as selector function interpolation 1`] = `
`;

exports[`styled composes 1`] = `
.glamor-1 {
.glamor-2 {
color: blue;
font-size: 32px;
}

<h1
className="glamor-0 legacy__class glamor-1"
className="glamor-0 glamor-1 legacy__class glamor-2"
scale={2}
>
hello world
Expand Down Expand Up @@ -216,7 +232,7 @@ exports[`styled composes based on props 2`] = `
`;

exports[`styled composes with objects 1`] = `
.glamor-1 {
.glamor-2 {
color: #333;
font-size: 32px;
height: 64px;
Expand All @@ -227,21 +243,21 @@ exports[`styled composes with objects 1`] = `
only screen and (-o-min-device-pixel-ratio: 1.5/1),
only screen and (min-resolution: 144dpi),
only screen and (min-resolution: 1.5dppx) {
.glamor-1 {
.glamor-2 {
font-size: 1.4323121856191332em;
}
}

<h1
className="glamor-0 legacy__class glamor-1"
className="glamor-0 glamor-1 legacy__class glamor-2"
scale={2}
>
hello world
</h1>
`;

exports[`styled composing components 1`] = `
.glamor-1 {
.glamor-3 {
color: green;
display: -webkit-box;
display: -ms-flexbox;
Expand All @@ -252,31 +268,31 @@ exports[`styled composing components 1`] = `
}

<button
className="glamor-0 glamor-1"
className="glamor-0 glamor-1 glamor-2 glamor-3"
>
hello world
</button>
`;

exports[`styled composition 1`] = `
.glamor-1 {
.glamor-2 {
font-size: 13.333333333333334px;
}

<h1
className="glamor-0 legacy__class glamor-1"
className="glamor-0 glamor-1 legacy__class glamor-2"
>
hello world
</h1>
`;

exports[`styled function in expression 1`] = `
.glamor-1 {
.glamor-2 {
font-size: 40px;
}

<h1
className="glamor-0 legacy__class glamor-1"
className="glamor-0 glamor-1 legacy__class glamor-2"
scale={2}
>
hello world
Expand All @@ -297,6 +313,7 @@ exports[`styled handles more than 10 dynamic properties 1`] = `
z-index: 100;
font-size: 18px;
text-align: center;
border-left: blue;
}

<h1
Expand All @@ -307,7 +324,7 @@ exports[`styled handles more than 10 dynamic properties 1`] = `
`;

exports[`styled higher order component 1`] = `
.glamor-1 {
.glamor-2 {
font-size: 20px;
background-color: '#343a40';
-webkit-box-orient: vertical;
Expand All @@ -317,7 +334,7 @@ exports[`styled higher order component 1`] = `
}

<div
className="glamor-0 glamor-1"
className="glamor-0 glamor-1 glamor-2"
/>
`;

Expand Down Expand Up @@ -520,15 +537,15 @@ exports[`styled random object expression 1`] = `
`;

exports[`styled themes 1`] = `
.glamor-1 {
.glamor-3 {
background-color: #ffd43b;
color: #8c81d8;
height: 64px;
font-size: 32px;
}

<span
className="glamor-0 glamor-1"
className="glamor-0 glamor-1 glamor-2 glamor-3"
Copy link
Member Author

Choose a reason for hiding this comment

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

@mitchellhamilton I'm not sure how to interpret these snapshot diffs. Do they look right to you?

Copy link
Member

Choose a reason for hiding this comment

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

They don't, there should only be two classes on each component.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the actual output on a web page. Maybe this will have a clue.

image

Copy link
Member Author

@tkh44 tkh44 Aug 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Notice the difference in the length of the hash.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is due to wrapping the components with withTheme I believe. I tested this out on the browser and it works as expected.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's correct because they're wrapped in withTheme so more components are rendered and the class is added for using components as selectors.

Now that I think about it, we might want to always add the class for using components as selectors because in this example OneComponent will have color: yellow; when rendered inside of SomeComponent but AnotherComponent will not.

const OneComponent = styled.div`
  display: flex;
`
const AnotherComponent = styled(OneComponent)`
  background-color: hotpink;
`
const SomeComponent = styled.div`
  ${OneComponent} {
    color: yellow;
  }
`

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think I should hold up merging this for that fix or is it something we can separate?

Copy link
Member

Choose a reason for hiding this comment

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

I already pushed a fix to this branch 😀

scale={2}
>
hello world
Expand Down
Loading