vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.99k stars 26.7k forks source link

Link with activeClassName #141

Closed sedubois closed 7 years ago

sedubois commented 7 years ago

Similar to react-router's Link, it would be nice to be able to customize CSS when a link is active, e.g with an activeClassName property.

nkzawa commented 7 years ago

duplicate of https://github.com/zeit/next.js/issues/127

sedubois commented 7 years ago

This isn't a duplicate, right now there is no way to style a link specifically when a route is active. React-router provides this in 147 + 233 LOCs. Although it's possible to take shortcuts thankfully 😄

nkzawa commented 7 years ago

ohhh, sorry!

It might be nice to have the activeClassName property, but you can manually detect if a route is active and set any class to the Link anyway ?

luisrudge commented 7 years ago

this would be super nice to have indeed!

sedubois commented 7 years ago

@nkzawa AFAIK detecting that requires 2 things:

Would be great to have Next take care of this instead of every user reimplementing it.

viankakrisna commented 7 years ago

Or maybe a withLocation HOC. And wrap Link with that HOC. Then import that instead of next/link

arunoda commented 7 years ago

One way to support is provide a way to get current state of the router using next/router. But it's not working on the Server.

So, what ever thing we need to done inside a componentDidMount hook. @nkzawa If we allow next/router on the server (Just for SSR) this would be great. Is there's any chance we could support it?

timneutkens commented 7 years ago

I'm thinking it might be implemented using componentDidMount since it's only executed client side. I'm not sure if it's executed every route change though.

Edit: seems like Arunoda thought the same thing above 👍

ghost commented 7 years ago

I think this would be a very good addition to the Links, I wonder if Zeit did it on zeit.co the way @mallendeo described

hieuhlc commented 7 years ago

Hey guys is this feature currently still developing?

arunoda commented 7 years ago

I think this is pretty easy to do without core support.

hieuhlc commented 7 years ago

I tried adding active classname in componentDidMount but it doesn't seem right. At first render active classname does not occur.

mtford90 commented 7 years ago

@arunoda do you have a good example of how to implement this given that it's pretty easy?

I use query parameters within my app and I've had to do some fairly ugly stuff to get the active class names to render the same on both client & server. Core support would be much nicer.

andrewmclagan commented 7 years ago

The main issue is its not only <Link /> that needs this functionality. other components such as custom buttons and such also need to know if they have an active route... I really really think this should be a utility attached to Router

<Component isActive={Router.isActive('/path/to/check')} />

The idea of having to pass down the pathname from the top level Page component is a code smell. What if your component is nested 10 deep..? Dont we want to abstract SSR quirks from the dev, is that not the point of this framework?

<Link /> could attach a active class automatically?

import Router from 'next/router';

const isActiveRoute = route => {
  if (typeof window !== 'undefined' && window.document && window.document.createElement) {
    return Router.pathname === route;
  }
  // Okay, now how to get the current route on the server?
  // assuming we would need to access the `req` object through a middleware.
  // return req && req.url;
}

export default isActiveRoute;

Work around

We currently use react context to get around this spaghetti nightmare.

// Page.js
import React from 'react';
import PropTypes from 'prop-types';
import Button from '../components/Button';

export default class Page extends React.Component {

  static getInitialProps ({ pathname, req }) {
    return { pathname: req && req.url || pathname  };  
  }

  static childContextTypes = {
    pathname: PropTypes.string,
  }  

  getChildContext() {
    return { pathname: this.props.pathname };
  }    

  render () {

    return (
      <div>
        <div>
          <div>
            <div>
              <Button />
            </div>
          </div>
        </div>
      </div>
    )
  }
}

And the deeply nested component that needs knowledge

import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

export default class Button extends React.Component {

  static contextTypes = {
    pathname: PropTypes.string,
  }  

  render() {

    const visible = this.context.pathname !== '/';

    return (
      <button className={classNames('btn-search', { visible })}>
        My Button
      </button>      
    );
  }
}

Not ideal...

steida commented 7 years ago

@andrewmclagan It's not so easy.

This usage can't handle URL change:

<Component isActive={Router.isActive('/path/to/check')} />

URL changes, so whenever we need to handle URL change, we need higher order component to enforce rerender. Something like injectUrl(PageHeader)

andrewmclagan commented 7 years ago

@steida yeah i figured that would be the case.

What about a wrapping component that uses context (as in my example above) that passes a className or isActive property to the child component?

I guess the main issue is keeping a codebase DRY is hard having to pass context down from every single page to do such a simple thing as active link. I really believe its a responsibility of the framework?

ghost commented 7 years ago

fwiw I still think this should be built into next/link

steida commented 7 years ago

I think this is pretty easy to do without core support.

Do you have a good example of how to implement this given that it's pretty easy?

Use the title, Luke.

That's how I am doing it in Este.

// @flow
import A from './a';
import Box from './box';

const HeaderA = ({ children, href, title }) => (
  <A
    backgroundColor="primary"
    bold
    color="white"
    href={href}
    isActive={title === children}
    paddingHorizontal={0.5}
    paddingVertical={0.5}
    prefetch
  >
    {children}
  </A>
);

type PageHeaderProps = { title: string };

const PageHeader = ({ title }: PageHeaderProps) => (
  <Box
    backgroundColor="primary"
    flexDirection="row"
    flexWrap="wrap"
    marginVertical={0.5}
    paddingHorizontal={0.5}
  >
    <HeaderA href="/" title={title}>Home</HeaderA>
    <HeaderA href="/about" title={title}>About</HeaderA>
  </Box>
);

export default PageHeader;

I think this is the right pattern. Not all links in an app must have an active state. Route change watchers are hard to implement and break clear data flow.

UPDATE: Maybe the example isn't obvious enough. I meant that we have to generate page titles anyway, and we can use generated page titles to match active route without sophisticated pathname parsing nor route change watching.

ayazhussein commented 7 years ago

The solution I found (might not be the best one) is to use path-to-regexp classnames packages and I made a util function that checks if the route is active by passing the props to my nav component...

// ./lib/activePath.js
import pathToRegexp from "path-to-regexp";

export function activePath(currentPath, path , options) {
  const regexPath = pathToRegexp(path, options);

  const result = regexPath.exec(currentPath);
  return result;
}
// ./component/Header.js
import React, { Component } from 'react';
import Link from "next/link";
import { activePath } from "../lib/activePath";
import classnames from "classnames";
class Header extends Component {
  render() {
    const { url } = this.props;
    const { pathname } = url;
    return (
      <header>
        <nav className="navbar navbar-toggleable-md navbar-light bg-faded">
          <button className="navbar-toggler navbar-toggler-right" type="button" data-toggle="collapse" data-target="#navbarNavAltMarkup" aria-controls="navbarNavAltMarkup" aria-expanded="false" aria-label="Toggle navigation">
            <span className="navbar-toggler-icon" />
          </button>
          <a className="navbar-brand">Title</a>
          <div className="collapse navbar-collapse" id="navbarNavAltMarkup">
            <div className="navbar-nav">
              <Link href="/">
                <a className={classnames("nav-item nav-link", { active: activePath(pathname, "/", { strict: true }) })}>Home <span className="sr-only">(current)</span></a>
              </Link>
              <Link href="/about">
                <a className={classnames("nav-item nav-link", { active: activePath(pathname, "/about", { strict: true }) })} >About</a>
              </Link>
              <Link href="/about/us">
                <a className={classnames("nav-item nav-link", { active: activePath(pathname, "/about/us", { strict: true }) })} >About US</a>
              </Link>
              <Link href="/contact">
                <a className={classnames("nav-item nav-link", { active: activePath(pathname, "/contact", { strict: true }) })} >Contact</a>
              </Link>
            </div>
          </div>
        </nav>
      </header >
    );
  }
}

export default Header;

the header is in my layout component so I have to pass the props to it on every page so that the header would have access to url props...

I'm open to other solutions too :D

arunoda commented 7 years ago

Guys, we are going to implement a new API to make these kind of functionalities simple. See: https://github.com/zeit/next.js/issues/2032

andrewmclagan commented 7 years ago

@arunoda Thanks I feel this was always going to be the right direction. We basically took the HOC route ourselves.

postor commented 7 years ago

hi guys, I made it using next-routes which makes one route define for both server side and client side, example here https://hello-next-lhafgwnfxp.now.sh/ and code here https://github.com/postor/nextjs-boilerplate

timneutkens commented 7 years ago

We'll merge the withRoute HOC after v3 has been released. Then this will be possible. https://github.com/zeit/next.js/pull/2352