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

css prop does not override if provided without enclosing { } #61

Closed
synchronos-t opened this issue Mar 19, 2019 · 17 comments · Fixed by #68
Closed

css prop does not override if provided without enclosing { } #61

synchronos-t opened this issue Mar 19, 2019 · 17 comments · Fixed by #68
Labels

Comments

@synchronos-t
Copy link

This is a bug report. react-spinners version 0.5.3.

The css prop is documented to override the default css, but it does not actually do this, if used like in the documentation, for example like this in my case:

  const cssOverride = css`
    margin: 20% auto 40%;
    display: block;
  `;

  const loader = (
    <ClipLoader
      css={cssOverride}
    />
  );

The display: block; is correctly added, but in a way that it does not override the default display: inline-block (the respective <style data-emotion="css"></style> is added earlier in the DOM).

However, if I enclose the css string in { } inside the template literal, it does override:

  const cssOverride = css`{
    margin: 20% auto 40%;
    display: block;
  }`;

This seems to be the way the default styles are written as well. I'm not experienced with emotion-js, so I cannot explain why that makes the object precedence work as intended, but it seems that's how it works.

@tbaustin
Copy link

Has this been fixed? I am not seeing my styles what so ever even with the { }

<RingLoader 
	css={css`{
		width: 300px;
		margin: 30px auto 0px auto;
		display: block;
		border-color: #333;
	}`}
	sizeUnit={`px`}
	size={150}
	color={`#fff`}
/>

----

<RingLoader 
	css={css`
		width: 300px;
		margin: 30px auto 0px auto;
		display: block;
		border-color: #333;
	`}
	sizeUnit={`px`}
	size={150}
	color={`#fff`}
/>

Neither of these are working. @davidhu2000

@davidhu2000
Copy link
Owner

I see the issue now.

This actually do not override the existing styles.

return this.props.css ? css`${wrapper};${this.props.css}` : wrapper;

I need to pass the new styles in as an array

<div css={[this.wrapper(), css]}>

That will actually override the old style. Getting a fix for this soon.

@davidhu2000
Copy link
Owner

davidhu2000 commented Jul 15, 2019

@tbaustin @synchronos-t can you check v0.5.12 and see if the issue has been resolved?

@bbmoz
Copy link

bbmoz commented Jul 18, 2019

@davidhu2000 I'm on v0.5.12, and I don't see the styles overwriting. I used both methods as described by @tbaustin .

@davidhu2000
Copy link
Owner

@bbmoz can you post the exact code you used? so i can try to reproduce

@bbmoz
Copy link

bbmoz commented Jul 19, 2019

Here is the code I used:

<PacmanLoader
  color='yellow'
  loading
  margin='2px'
  size={20}
  sizeUnit='px'
  css={css`
    margin-top: 24px;
    left: 50%;
    transform: translateX(-40px);
  `}
/>

There is more to the above to provide context, but in terms of code relevant to the library, this is it. Thanks for the help!

@davidhu2000
Copy link
Owner

Screen Shot 2019-07-18 at 9 25 28 PM

when i tried your code, it's working fine for me. Please note that the override only works on the wrapper. It will be on this class css-1bfp83e in the image

@eoghanmccarthy
Copy link

This issue still persists in 0.6.1

@stekyne
Copy link

stekyne commented Apr 16, 2020

Still seeing this in 0.81 even.

@krstic-duci
Copy link

Still seeing this in 0.9.0 even.

@davidhu2000
Copy link
Owner

reopening the issue for further investigation.

Can you provide a code sample to help reproduce?

@davidhu2000 davidhu2000 reopened this Jul 23, 2020
@krstic-duci
Copy link

krstic-duci commented Jul 27, 2020

Hi,

Please have a look at this codesandbox example.

Cheers

@davidhu2000 davidhu2000 added this to Todo in v0.10.0 Jul 28, 2020
@davidhu2000
Copy link
Owner

davidhu2000 commented Dec 27, 2020

so taking off this code fixes the overriding issue.

/** @jsx jsx */

However, without that, now this code no longer works

          <section
            css={css`
              font-size: 26px;
            `}
          >

I see an output

You have tried to stringify object returned from css function. It isn't supposed to be used directly (e.g. as value of the className prop), but rather handed to emotion so it can handle it (e.g. as value of css prop).

Adding a comment to document this issue. Not sure how to solve it right now.

@davidhu2000
Copy link
Owner

something worth noting is all the loaders include

/** @jsx jsx */

https://github.com/davidhu2000/react-spinners/blob/master/src/CircleLoader.tsx#L1

I wonder if that need to be removed and use @emotion/babel-preset-css-prop instead.

@davidhu2000 davidhu2000 removed this from Todo in v0.10.0 Dec 27, 2020
@ghost
Copy link

ghost commented Jan 9, 2021

Is this fixed?

@mountwebs
Copy link

This is still an issue for me.

@davidhu2000
Copy link
Owner

this issue should be fixed in 0.13.0-alpha.5, so closing this for now. feel free to reopen if there are issues

@davidhu2000 davidhu2000 added this to Done in 0.13 May 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
0.13
Done
v0.10.x
Awaiting triage
Development

Successfully merging a pull request may close this issue.

8 participants