reach / router

https://reach.tech/router
MIT License
6.89k stars 327 forks source link

Not possible to pass props to (hence style) the nested div wrappers #145

Open adamscybot opened 6 years ago

adamscybot commented 6 years ago

Firstly, I totally agree with the decision in https://github.com/reach/router/issues/63 to make it so the <div> wrappers can not be disabled. This is an accessible router after all.

However, it does pose a problem for layouts that require these elements to be styled, especially in flex box layouts.

For the top level wrapper created by <Router> the problem is already solved since props are spread to the wrapper. This means we can use the style or className attribute like normal.

However, it seems there is no way to pass props to the div's that are more deeply nested (presumably around each route component). The result is it is not easy to style these without resulting to dirty-looking child or attribute CSS selectors. I think we should be able to style these divs by finding a way to pass the style or className attribute. This will also maintain consistency with the top level div.

I can probably submit a PR for this if you feel it is useful, but I'd like to confirm the API. The options I can think of are:

A) A new prop to pass in the HTML attrs to spread

In this option, we change the API of route components to accept a new prop that accepts an object. That object is spread as the props to the DIV. For example:

<Router>
    <MyRoute path="/home" wrapperProps={{ className: 'my-class' }} />
</Router>

B) "Component" render prop

My preferred option, we reserve the component prop in the same way we do for the <Router> component.

I think this is nice as it woul work quite neatly with component based CSS-in-JS solutions

<Router>
    <MyRoute path="/home" component={wrapperProps=> <div className="my-class" {...wrapperProps}  /> } />
</Router>

Would a PR be accepted?

zheeeng commented 6 years ago

+1

HenriBeck commented 5 years ago

Any updates on this?

PiotrGrzechnik commented 5 years ago

I know it's just workaround but if you set prop primary to false you can grab all these divs and add some class:

const elements = document.querySelectorAll('div:not([class]):not([id])'); Array.from(elements, item => item.classList.add('emptyRouterDivs'));

joewood commented 5 years ago

I like the suggestions in the OP. In trying to use these wrapper routes in a css-grid, a workaround I used was to use styled components CSS on the parent grid and the & > div:nth-child(2) selector to set the grid cell style for each wrapper.

cdock1029 commented 5 years ago

This allows me to pass className to wrapper div, and props to child routes (when using nested routes).

{children &&
  React.cloneElement(children, {
    className: 'flex-1',
    children: React.Children.map(children.props.children, child => {
      return React.cloneElement(child, {
        customer,
        setCustomer,
      })
    }),
  })}

Found here: https://github.com/reach/router/issues/163#issuecomment-451798848

jcmoore0 commented 5 years ago

Any update on this?
Reach router is fantastic but this problem is a stopper for me using flex layouts.
I like both options above by @adamscybot. Thanks,

trevorblades commented 4 years ago

I was able to get around this by using the following technique. I set the Router component's primary prop to false, and component to Fragment. I then force the route children (automatically wrapped in a Router component) to take the same component prop.

import React,  {Fragment} from 'react';
import {Router} from '@reach/router';

function Outer({children}) {
  return (
    <>
      <h1>App title</h1>
      // THIS IS THE IMPORTANT PART ⬇️
      // pass a component prop to the route children (actually a Router instance)
      {React.cloneElement(children, {component: Fragment})}
    </>
  );
}

function Inner() {
  return <h3>nested route</h3>;
}

function App() {
  // set primary to false and use a Fragment as the Router component prop
  return (
    <Router primary={false} component={Fragment}>
      <Outer path="/foo">
        <Inner path="/settings" />
      </Outer>
    </Router>
  );
}

Check out the live demo of this technique here: https://yusel.codesandbox.io/foo/settings Code here: https://codesandbox.io/s/reachrouter-and-flex-yusel?file=/src/App.js

jcmoore0 commented 4 years ago

@trevorblades - Really cool solution. Thanks for sharing.

tcrossland commented 4 years ago

See also https://github.com/reach/router/issues/163#issuecomment-665338963

wasurocks commented 3 years ago

I've been wasting a few hours on this now (since I need to use flex) and @trevorblades 's solution works. Thank you.

welkinwong commented 3 years ago

@trevorblades it works well! thanks

sonlichao commented 3 years ago

@trevorblades Really cool solution. Thanks for sharing. But, I get some warning like this. How can I solute it.

スクリーンショット 2021-07-08 16 45 34 スクリーンショット 2021-07-08 16 45 51
trevorblades commented 3 years ago

@sonlichao let's see how your code looks. Seems like you're passing a style prop and a ref to the route directly. You could try adding your style prop to the parent element within the route component, but it's hard to suggest anything without seeing your implementation.

sonlichao commented 3 years ago

<Router component={Fragment} primary={false}>

It worked for me by setting primary as false. @ trevorblades Thank you very much.