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

Adding support for styling and rendering components on VR platform #755

Closed
nitin42 opened this issue Jul 7, 2018 · 8 comments
Closed

Adding support for styling and rendering components on VR platform #755

nitin42 opened this issue Jul 7, 2018 · 8 comments

Comments

@nitin42
Copy link
Contributor

nitin42 commented Jul 7, 2018

  • emotion version:
  • react version:

Relevant code.

What you did:

What happened:

Reproduction:

Problem description:

Using @emotion/primitives with react-360 to style VR apps across the platforms. But, @emotion/primitives depends on react-primitives which when installed in react-360 project, gives an error that

couldn't resolve module react-vr

But when we try to install react-vr, it conflicts with react-360.

Suggested solution:

We can detach the css function from @emotion/primitives and create a separate package for it, and then use it according to the environment.

Something like this:

// Emotion primitives

import { createCssFunction } from 'emotion-primitive-css'

import { StyleSheet } from 'react-primitives'

// This will use StyleSheet method for styling components in react-native, Web and Sketch
const css = createCssFunction(StyleSheet)

export default css
// Emotion VR

import { createCssFunction } from 'emotion-primitive-css'

import { StyleSheet } from 'react-360'

// This will use StyleSheet method for styling components in react-360
const css = createCssFunction(StyleSheet)

export default css
@emmatown
Copy link
Member

emmatown commented Jul 7, 2018

I'm good with adding something like this though I'd rather call it createCss and have a createStyled function like this and call the package @emotion/primitives-core

// @flow
import * as React from 'react'

export function createCss(StyleSheet) {
  // ...
}

type Options = {
  getShouldForwardProp: (
    Component: React.ElementType
  ) => (prop: string) => boolean
}

let defaultPickTest = prop => prop !== 'theme' && prop !== 'innerRef'
export function createStyled(
  StyleSheet,
  { getShouldForwardProp = () => defaultPickTest }: Options = {}
) {
  // ...
}

And then it can be used in @emotion/primitives like this

// @flow
import * as React from 'react'
import { View, Image, Text, StyleSheet } from 'react-primitives'
import {
  testPickPropsOnPrimitiveComponent,
  testPickPropsOnOtherComponent
} from './test-props'
import { createStyled, createCss } from '@emotion/primitives-core'

function getShouldForwardProp(component: React.ElementType) {
  switch (component) {
    case View:
    case Text:
    case Image: {
      return testPickPropsOnPrimitiveComponent
    }
  }
  return testPickPropsOnOtherComponent
}

let styled = createStyled(StyleSheet, { getShouldForwardProp })

// ...assign View, Image and Text

export default styled

export let css = createCss(StyleSheet)

And we can create a new package, @emotion/native and it's implementation can be something like this

// @flow
import { StyleSheet } from 'react-native'
import { createStyled, createCss } from '@emotion/primitives-core'

let styled = createStyled(StyleSheet)

// ...assign all the components

export default styled

export let css = createCss(StyleSheet)

It'd be great if you could submit a PR for this!!

Just a note, the folder names for scoped packages should be the name after @emotion/

@nitin42
Copy link
Contributor Author

nitin42 commented Jul 7, 2018

Sure! I'll go ahead and create a PR. I'll add @emotion/native and @emotion/360 with this PR if that's sounds good to you!

@emmatown
Copy link
Member

emmatown commented Jul 7, 2018

I'm not totally sure about @emotion/360 since it's a pretty rare use case and if we add it then people might want particular ones for things like sketch, react-native-web and maybe others, though I still think @emotion/native is a good idea though since it's generally the most popular use case and will have more component shorthands.

Maybe instead of adding @emotion/360, react-primitives could be changed to use react-360?

@nitin42
Copy link
Contributor Author

nitin42 commented Jul 7, 2018

Hm.. I think you're right about this! I'll make changes for @emotion/native. Also few things -

  • StyleSheet method in react-360 uses signatures similar to react-native. So @emotion/native should work for react-360.

  • we cannot change react-primitives for adding react-360 support 😅

@brunolemos
Copy link

The thing is that react-primitives is outdated, right?
They should rename react-vr to react-360. There's an open pr for that.

Maybe you should contact maintainers.

@nitin42
Copy link
Contributor Author

nitin42 commented Jul 8, 2018

Haha react-primitives is way behind. It has to do so much work on the core, new primitives, animations and bug fixes. I'll try to contact the maintainers.

@stale
Copy link

stale bot commented Sep 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 11, 2018
@nitin42 nitin42 closed this as completed Sep 12, 2018
@mathieudutour
Copy link

[email protected] now requires react-360 instead of react-vr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants