Make checkStyles optional

[denkristoffer]

👋

I'd like to propose making running checkStyles() optional with some kind of opt-out. My thinking is that this will make @reach/ui easier to adopt for projects that want to benefit from the accessibility features but want to use their own styling without having to override unnecessary defaults.

As it is now the warning can be ignored since it doesn't show up in production, or it can be removed by manually setting the CSS variables used in the check, but the first is a bad idea and the second would break if the check is changed. An option to remove the warning "properly" would be nice 🙂 What do you think?

[lee-reinhardt]

I'd also love to hear if anyone has found a way to handle this situation for tests (the styles are actually included)?

[jkdowdle]

I am also having the same issue as @lee-reinhardt while testing my react components that are using reach-ui in create-react-app.

Here is my work around for now.

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

But I think a solution like @denkristoffer suggested would be helpful as I also had an issue trying to override the data attribute selectors with styled-components. I may have been making a mistake, but for now I just threw my custom styles into a css file.

[lee-reinhardt]

@jkdowdle That is probably a saner solution (for testing) than what I ended up doing. I used this situation as an excuse to use patch-package. My patch looks like this:

patch-package
--- a/node_modules/@reach/utils/index.js
+++ b/node_modules/@reach/utils/index.js
@@ -5,7 +5,7 @@ var checkedPkgs = {};
 
 var checkStyles = function checkStyles() {};
 
-if (process.env.NODE_ENV !== "production") {
+if (false && process.env.NODE_ENV !== "production") {
   exports.checkStyles = checkStyles = function checkStyles(pkg) {
     // only check once per package
     if (checkedPkgs[pkg]) return;

Obviously not a long-term solution, but like I said, it was a fun reason to use it.

---

For styling the components, this is an example of what I do (using emotion, but basically the same as styled-components):

import React, { Component } from 'react'
import styled, { css } from 'react-emotion'
import { space } from 'styled-system'
import {
  Menu as ReachMenu,
  MenuList as ReachMenuList,
  MenuButton as ReachMenuButton,
  MenuItem as ReachMenuItem,
  MenuLink as ReachMenuLink,
} from '@reach/menu-button'
import { Theme } from '@ui/theme'
import { withButtonStyle } from '@ui/Button'

interface StyledProps {
  theme: Theme
}

const menuListStyle = ({ theme }: StyledProps) => css`
  &[data-reach-menu-list] {
    padding: unset;
    border: unset;
    font-size: unset;
  }

  width: 200px;
  margin-top: 10px;
  border-radius: 3px;
  overflow-y: scroll;
  box-shadow: ${theme.shadows.large};
`

const menuItemStyle = ({ theme }: StyledProps) => css`
  &[data-reach-menu-item] {
    font-size: 90%;
    padding: 16px;
  }

  &[data-reach-menu-item][data-selected] {
    color: unset;
    background-color: ${theme.colors.lightblue};
  }

  &[data-reach-menu-item]:hover {
    color: unset;
    background-color: ${theme.colors.lightblue};
  }
`

class Menu extends Component {
  static List: any
  static Button: any
  static Item: any
  static Link: any

  render() {
    return <ReachMenu>{this.props.children}</ReachMenu>
  }
}

Menu.List = styled(ReachMenuList)(menuListStyle)
Menu.Button = withButtonStyle(ReachMenuButton)
Menu.Item = styled(ReachMenuItem)(menuItemStyle, space)
Menu.Link = styled(ReachMenuLink)(menuItemStyle, space)

export default Menu

[jkdowdle]

@lee-reinhardt

Ah! Thank you so much on that styling issue!

I was getting the sass syntax wrong trying to do it as if it was a hover or focus.

const MenuList = styled(MenuListPrimative)`
  /* Notice the colon (:) shouldn't be included */ 
  &:[data-reach-menu-list] { 
    background: purple;
  }

Then I hadn't heard of the patch package before, but I may consider that.

A somewhat reasonable change might be excluding the check in production as well as test environments? Though I am not sure if that would also affect something like storybook.

Thank you so much for the suggestions!

[ryanflorence]

Yeah I'd like to do something here. The intent is to have some basic styles for starting out, but you should be able to just skip the included styles completely if you want w/o a bunch of warnings.

For now I suggest adding the css variables in development.

In the future it'll probably be something like:

import Thing from "@reach/thing/unstyled"

Or the inverse:

import Thing from "@reach/thing/styled"

and that'd assume you've got a bundler that can import styles.

I'm going to close this as its something that's very much on my radar but has an okay workaround for now (css variables in dev).