search

HomeServicesOfAmerica / styled-material-components

Styled Components implementation of Material Design
Other
71 stars 51 forks source link

fix(Column): fix breaking changes from ScreenSizeContext #260

Closed petegivens closed 6 years ago

petegivens commented 6 years ago

This PR fixes a bug that was breaking most of the SMC pages where Column was not able to be converted to a styled component due to some effect of returning a forwardRef instead of the actual component. Per @brad-decker, the forwardRef was only a failsafe so we're removing it for now.

martahj commented 6 years ago

Cool!

In terms of the forwardRef, I've opened a PR that would add styled-components support for these https://github.com/styled-components/styled-components/pull/1658. It was working via symlink but who knows what they'll think of it. It doesn't sound like the forwardRef is super important to the Column though so it sounds like we should move forward with your PR regardless, as this is a blocker for all SMC work.

I'd ask you to post a Now link but that's not possible right now.. can you just verbally verify that it's working locally? 😉

petegivens commented 6 years ago

@martahj lol yeah, I tried to build one, but...

I have verified that all most of the SMC pages are working locally.

Super cool that you already submitted a PR to SC!!

brad-decker commented 6 years ago

Who's this guy that keeps breaking things??? <_<

Good work @petegivens cleaning this up, and woohoo @martahj hope that gets merged!

petegivens commented 6 years ago

Actually, hold off on merging for a minute please. Just noticed a bug with the snackbar.

petegivens commented 6 years ago

@brad-decker @martahj There's another issue cropping up. All I've determined for sure so far is that Snackbar breaks if we pass a ref into it from ScreenSizeContext. Flex-Grid breaks if ScreenSizeContext doesn't pass it a ref. I've tried subbing innerRef instead but it made no difference. The error is:

TypeError: Cannot add property current, object is not extensible
    at safelyDetachRef (http://localhost:8081/_next/1522889688587/commons.js:9650:21)
    ...etc

Looking into it more now.

petegivens commented 6 years ago

I was getting the above-mentioned error from the dev server and Warning: Unexpected ref object provided for Styled(SnackbarComponent). Use either a ref-setter function or React.createRef(). in the console.

I resolved by removing the ref that ScreenSizeContext was passing down, but I'm not sure why it wasn't being recognized as a valid ref.

Additionally, withRowState is throwing this warning: Stateless function components cannot be given refs. Attempts to access this ref will fail.

Just going to remove the refs from both for now, what do you think?

martahj commented 6 years ago

@petegivens Removing the refs sounds fine to me