Add ability to use with router links

[bryceosterhaus]

Randomly thought about this today. We need to provide a way to use a custom anchor component for anchors. This might be able to be achieved via context but I'm not super positive. This thought came from thinking "how would I use clay with react-router?". We would need to pass <Link to="/"> or something.

[matuzalemsteles]

Hmm, I'm not sure if I understood well about creating a custom anchor component for anchors. I can't imagine what that would be like via the Context API.

[bryceosterhaus]

I'm not totally sure if context would work either, but we for sure need some mechanism that allows for a package's custom Link type component to be used as anchors instead of just <a>. Unless I am mistaken on this? I would think this would be a pretty common use case where someone consuming clay with react-router needs to make sure that a component like Nav uses Link instead of a. Otherwise the routing wont work properly.

[matuzalemsteles]

Hmm, I was looking at the implementation of react-router, devs can pass it the component that only renders the markup https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/Link.js#L43.

Example:

<Link component={ClayLink} to="/">Test</Link>

[bryceosterhaus]

Right, but if a user wanted to use ClayNavigation.Breadcrumb how would they ensure that the anchor tags run via the router? Right now if you dropped that component into a react-router app, their hrefs in the breadcrumb component would cause full page reloads rather than utilize the frontend SPA routing via react-router. So ideally we would provide some sort of context API that our components would opt to use a custom anchor where we use anchors internally

[matuzalemsteles]

For such cases dev should use the onClick callback which Breadcrumb will render a ClayButton in place of the anchor so that it can control. I am a little hesitant about this because we would be stuck only with the react-router (although it is the most popular) I don't want us to create a coupling. But I'm still lost what this context API would look like, I can't see it so clearly.

[bryceosterhaus]

Oh I think I may have mis-explained. I don't want to couple tightly with react-router, I am just using that library as an example.

I also wouldn't want to force people to use onClick for this use case since having the accessibility of an anchor tag is very helpful.

Hypothetically, I imagine an API looking like:

import {Link} from 'react-router';

const ClayLinkContext = React.createContext('a');

const ClayLink = () => {
    const LinkComponent = React.useContext(ClayLinkContext);

    return <LinkComponent href={href} />;
};

const MyApp = () => (
    <div>
        <ClayLink href="/foo/bar" />
    </div>
);

<ClayLinkContext.Provider value={Link}>
    <MyApp />
</ClayLinkContext.Provider>;

Does this make sense to you?

[matuzalemsteles]

Ohh now it's clear. I think it makes sense. Liked it!

I think we can slightly improve the use of our providers and create a ClayManager to manage them all or just be Clay's sole context. Something like that:

//...
const config = {
    spritemap: '...',
    linkComponent: Link
};
//...
<ClayManager value={config}>
   <MyApp />
</ClayManager>

Thus would shrink the nested providers in the application, this can easily grow. What do you think?

[bryceosterhaus]

@matuzalemsteles I think we can do both. We would provide a specific ClayLink context, but then also we could provide a ClayContext that wraps all of our context providers.

[matuzalemsteles]

Hmm, it seems a little weird to have so many separate contexts, it will be quite common to see people using most of our components so it will be natural for them to use practically all contexts, so I don't know if it would be beneficial to provide multiple contexts and it seems a bit strange to add multiple contexts. I think having just one context seems wiser to me.

[bryceosterhaus]

I am proposing both, this way if a user only wants to use a single component and context, they aren't required to use a shared context.

For Example

// #1
<ClayIconContext.Provider value="/path/to/spritemap">
    <ClayLinkContext.Provider value={Link}>
        <ClayStoreContext.Provider value={{data}}>
            <MyApp />
        </ClayStoreContext.Provider>
    </ClayLinkContext.Provider>
</ClayIconContext.Provider>

// #2
<ClayContext.Provider
    value={{
        linkTag: Link,
        spritemap: "/path/to/spritemap",
        store: data
    }}
>
    <MyApp />
</ClayContext.Provider>

By allowing for both options, we allow users to use a single context rather than the global context. If a user only intends to use a few icons and a few links, I think it'd be helpful to allow the option to use that context separately. Alternatively, we also can compose all of our contexts together and offer a global context for clay as a whole, this would likely be used in a large app that takes advantage of multiple clay packages.

By allowing single context providers, we also give a more simple ability to override, for example if we have a deeply nested component that needs a different spritemap, we can do something like this...

// App
<ClayContext.Provider
    value={{
        linkTag: Link,
        spritemap: "/path/to/spritemap",
        store: data
    }}
>
    <div>
        <ClayManagementToolbar />

        <ClayList />

                 // Imagine this is deeply nested throughout other files...
        <ClayIconContext.Provider value="/modal/spritemap">
            <ClayModal />
        </ClayIconContext.Provider>
    </div>
</ClayContext.Provider>

I think if we only provide a global context, this makes it slightly more tricky in re-composing the global context object.

What do you think? I am primarily proposing this for developer convenience and not requiring a global context for every component.

TLDR; offer both a context for the single package level ClayIconContext and also a global scale ClayContext.

[matuzalemsteles]

Hmm, this is interesting the second case, I think we can test these two, although I still don't see a very large value. Having only one global context even if it is only for a single case does not make it expensive, but using a global context and then the unique context for the second case I think it sometimes ends up being more expensive to manage everything, although in the global context we are only aggregating.

It would be interesting if we can gain insights into the use of these APIs, to understand how people use components to draw things that have not been used so much and perhaps even design something better based on data, thinking of something like Analytics for components in the dev environment. Just an idea I had 🙂.

[wincent]

<ClayLinkContext.Provider value={Link}>
  <MyApp />
</ClayLinkContext.Provider>;

So I think the only "gotcha" here is that we need to make some assumptions about the props that the link component takes. If the user wants to use a component from "some-other-router" package that doesn't match that API, then they'd need to make a wrapper that adapts their link for use in Clay; for example, imagine package takes a to prop but we pass href:

const Link = (href) => {
    return <SomeOtherRouter.link to={href} />;
};

On the subject of context (single vs multiple) there is probably a sweet spot in there somewhere. If you use one global context for everything, then everything ends up depending on it, which means that every time the context changes, everything will re-render, which would be bad (some related reading). On the other end, if you have a zillion tiny contexts for specific things, you have the mess of dealing with lots of little things. "1" is probably not the right number, but neither is "100"; the correct value is somewhere in there though. 😂

[bryceosterhaus]

@wincent in regards to the "gotcha", I think it'd be fine to assume that the value will be a component with an explicit set of props(likely anchor attrs), and then to use it properly a user would have to create a component like you mentions above.

Also, good point about the the "in-between". This is partially the reason I like the ability to add both a single global one, and then multiple small ones. By having the multiple small ones, we would allow devs to compose any other sort of context needed.

[bryceosterhaus]

Fix: https://github.com/liferay/clay/pull/2281

[erickakoyama]

I was looking into this exact feature just now. We are migrating to using Clay 3.x React components and we are using react-router. We will not be able to adopt any of the new Clay components that render links until there is a solution for supplying a custom Link component for these types of components.

[bryceosterhaus]

@matuzalemsteles should we close this with it being labeled unstable?

[matuzalemsteles]

I think it makes sense once we are implementing a test solution and the work has been done, we can soon create a new issue for us to make sure we should mark this API as stable or not.