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

Include styledBy in csskit package #14812

Closed
2 tasks done
yordis opened this issue Mar 10, 2019 · 5 comments
Closed
2 tasks done

Include styledBy in csskit package #14812

yordis opened this issue Mar 10, 2019 · 5 comments
Labels
new feature New feature or request package: styled-engine Specific to @mui/styled-engine

Comments

@yordis
Copy link
Contributor

yordis commented Mar 10, 2019

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

import { styledBy } from '@material-ui/styles'; expect styledBy to be defined.

Current Behavior 😯

styledBy is undefined.

Examples 🌈

import { styledBy } from '@material-ui/styles';

export const Something = styled('div')({
  flexGrow: 1,
  display: 'flex',
  justifyContent: styledBy('alignContent', {
    start: 'flex-start',
    end: 'flex-end',
    center: 'center',
  }),
});

Context 🔦

Hey folks, I know that you pretty much recommend this function and for the most part is one line function, as such:

const styledBy = (property, mapping) => props => mapping[props[property]];

But because it is such of small functionality with a huge benefit for me as a consumer of the library, I would prefer to have the function exposed.

My alternatives are, install another package or copy-paste the function in every project that I want to use it, which both of them are not as convenient as simple use it from material-ui/styles package which I am using anyway.

@oliviertassinari
Copy link
Member

@yordis I'm happy to expose this helper. I don't think that it should come from @material-ui/styles as it's more generic. Maybe from the @material-ui/csskit package that @n-batalha has started working on?

@oliviertassinari oliviertassinari added the new feature New feature or request label Mar 10, 2019
@yordis
Copy link
Contributor Author

yordis commented Mar 10, 2019

@oliviertassinari but that is another dependency I need to manage though. For what I can tell, csskit are some utils around generating CSS.

But this function is all about CSS-in-JS functionality.

I don't mind either way, but if feels odd installing another package just because of one function.

I am currently using (one of the non-material/core projects) material/style because of the built-in CSS-in-JS and this is why I would love to have this functionality included there since it is a common use case.

As generic as it gets I guess. I don't understand what do you mean by your statement since it seems generic enough.

This helper is not about creating specific CSS as csskit will do, it is about providing you easy integration with CSS-in-JS library like material/style."

Thoughts?

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 10, 2019

@yordis I have two comments. The first one is that csskit focuses on the needs people using CSS-in-JS might have (if you have a better name proposal, we would be happy to hear it). It will contain small helpers, I hope we can extract all the theme helpers into it.
We have a more opinionated CSS-in-JS helper: the system package. 1. It allows an atomic CSS like API, leveraging the theme values. 2. It can save you time when creating custom components.

The second comment is that it's critical to have the smallest possible style package. I fear adding helpers to it will divert us from this goal.

@yordis
Copy link
Contributor Author

yordis commented Mar 11, 2019

That makes sense. Well csskit then (I like that name)

@oliviertassinari oliviertassinari changed the title Include styledBy in styles package Include styledBy in csskit package Mar 11, 2019
@oliviertassinari oliviertassinari added the package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. label Apr 17, 2021
@oliviertassinari oliviertassinari added package: styled-engine Specific to @mui/styled-engine and removed package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. labels May 9, 2021
@oliviertassinari
Copy link
Member

I'm closing as we don't use this pattern in the source. So far, we have preferred to access the props at the style level (not the property one) and use an object to map:

export const Something = styled('div')((props) => ({
  flexGrow: 1,
  display: 'flex',
  justifyContent: {
    start: 'flex-start',
    end: 'flex-end',
    center: 'center',
  }[props.alignContent],
})); 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request package: styled-engine Specific to @mui/styled-engine
Projects
None yet
Development

No branches or pull requests

2 participants