Reach-ui: Remove warning when not including css ?

[UnbearableBear]

Hi, I am currently using styled components to write the styles without having to include any css from the package. I simply do write them this way:

import { DialogContent, DialogOverlay } from "@reach/dialog";

const StyledDialogOverlay = styled(DialogOverlay)`
  position: fixed;
  top: 0;
  right: 0;
  bottom: 0;
  left: 0;
  background: rgba(0, 0, 0, 0.5);
  overflow: auto;
`;
const StyledDialogContent = styled(DialogContent)`
  position: relative;
  margin: 10vh auto;
  padding: 2rem;
  width: 50vw;
  background: white;
  border-radius: ${box.borderRadius};
  outline: none;
`;

However, I do get a warning when doing so: @reach/dialog styles not found. Are you sure it is a good idea to do css detection in JS ?
It is kind of annoying to have this warning to pop up every time I run my tests :/

[rebolyte]

The warning message is just based on checking whether a style was applied to window (see here), so I got rid of the annoying message in my Jest unit tests with a mock:

// save reference
const { getComputedStyle } = window;

beforeAll(() => {
    window.getComputedStyle = jest.fn().mockReturnValue({
        getPropertyValue: jest.fn().mockReturnValue('1')
    });
});

afterAll(() => {
    window.getComputedStyle = getComputedStyle;
});

[raunofreiberg]

As a workaround, you can set the package CSS variable to silence it since that's how the warning is shown in the first place.

E.g. if you were using @reach/tooltip, you would do:

:root {
  --reach-tooltip: 1;
}

After setting this, the warning is no longer shown.

[gnapse]

I too would like to request that there should be a way to silence these off. The way we load styles is unfortunately not available in the test environment. However, we cannot rely on the above hack of mocking getComputedStyle (for reasons that are irrelevant here). So we're getting a bunch of these warnings in our tests.

[gnapse]

A bit less intrusive alternative to @rebolyte's workaround above:

// save reference
const { getComputedStyle } = window

beforeAll(() => {
    window.getComputedStyle = element =>
        element === document.body
            ? { getPropertyValue: () => "1" }
            : getComputedStyle(element)
})

afterAll(() => {
    window.getComputedStyle = getComputedStyle
})

This at least won't mess up with the calls to getComputedStyle made on elements other than the body (that's part of the reason why the workaround did not work for us).

[bradwestfall]

@UnbearableBear Let us know if those suggestions worked for you

[gnapse]

I disagree with having this issue closed. The above workarounds are kinda hacky, and the css warning, although useful for beginners, should be more easily turned off. It seems the tests scenario is pretty common.

[UnbearableBear]

Those suggestions could work, however I'm not a big fan of technical debt. So I simply accept the fact that this message is part of our project now 💃

[bradwestfall]

@gnapse No worries, I can re-open. We're starting to dedicate a lot of time to this lib so I was just going through and cleaning up so issues (labels and easy closes) to give @chancestrickland and @mjackson a head start. I don't have any deep thoughts on whether the issue should be closed. It seemed like it might. I'll re-open it and let them help you out when they can

[chaance]

@UnbearableBear @gnapse How about something like this in our style check that allows you to set a global override? This could be set in Babel/Jest/whatever.

if (!global.__REACH_DISABLE_STYLE_CHECK) {
  // continue check
}

We do want to keep the warning on by default. Reach is ultimately about accessible components, and styles are important for making components accessible. We still think overriding default styles is the best way to go but understand there will be good reasons to disable them if that's your butter.

[UnbearableBear]

I guess it's just me being stubborn but I'd take the bet that they are more people being annoyed by this message than people needing this message to realise they have no styles on their modal. I mean it's hard to miss the fact that you have no styles

[selbekk]

If you're using Jest, you can mock out the checkStyles function like so:

jest.mock('@reach/utils', () => ({
  ...jest.requireActual('@reach/utils'),
  checkStyles: jest.fn(),
}));

[vilchesalves]

A bit less intrusive alternative to @rebolyte's workaround above:

// save reference
const { getComputedStyle } = window

beforeAll(() => {
    window.getComputedStyle = element =>
        element === document.body
            ? { getPropertyValue: () => "1" }
            : getComputedStyle(element)
})

afterAll(() => {
    window.getComputedStyle = getComputedStyle
})

This at least won't mess up with the calls to getComputedStyle made on elements other than the body (that's part of the reason why the workaround did not work for us).

I'm still having the issue using @reach-ui/dialog so, expanding on that, you could check specifically for a --reach attribute. I'm using something like this in my jest-loadershim.js:

const originalGetComputedStyle = window.getComputedStyle;

window.getComputedStyle = (element) => (
  element === document.body
    ? {
      getPropertyValue: (prop) => (
        prop.includes('--reach')
          ? '1'
          : originalGetComputedStyle(document.body).getPropertyValue(prop)
      ),
    }
    : originalGetComputedStyle(element)
);