Link component

Description

<Link> is a component needed in all the themes. Even tho it’s not complex to create one tailored to the developer needs, we can provide a default one.

In my option, we need to make it super clear in the documentation that theme developers can create their own <Link> component with their own logic. We should provide examples of how to do so. The usage of this component should always be optional.

User Stories

As a theme developer
I want to import a default Link component
so that I don’t have to create it myself

Possible solution

Where should it live:

  • In the @frontity/components package, imported like this:
import Link from "@frontity/components/link";

const MyComp = () => {
  return <Link>...</Link>;
}
  • In the all the router packages, imported like this:
const MyComp = ({ libraries }) => {
  const Link = libraries.router.Link;
  return <Link>...</Link>;
}

I noticed that in @frontity/mars-theme the current behavior of the Link component prevents the user from opening links in a new tab when using a shortcut such as cmd + click. This should be contemplated. An easy solution, even if not exhaustive, is described in this post:

1 Like

Thanks @orballo.

Let’s add this user story for this:

As a final reader
I want to open links in new tabs when I cmd+click
so that I can use the site without hassle

1 Like

Another cool thing could be to add a warning if people forget to pass the link prop, like in this case: Link does not render href on the anchor tag

As a Frontity developer
I want to get a warning if my links won’t render a href
so that I can be sure that all my links work

I’m planning on giving this one a shot, seems like a good first ticket.

Do I need to create a issue on github? Or are we using this forum as the source of truth for tracking tickets/discussions?

I’m glad to hear that you’d like to give this a shot @nicholasio.oliveira :slightly_smiling_face:

The way we usually work is using the Feature Discussion in the forum to discuss first which functionalities should be included in the first version and how it should work, until we end up with an Implementation Proposal (like this one). From there, we start a Pull Request in GitHub for the code. We keep GitHub to talk about the code itself and the forum for the discussions related to the implementation.

So I guess the first step should be to restart this conversation about which functionalities the Link component should include and where it should live, in @frontity/components or in the router. It’d be great to hear your opinion as well! Any feedback here @dev-team ?

Awesome :slightly_smiling_face:

Yeah, feel free to cover as much User stories as you want, or even add more.

Also the PR doesn’t need to contain all of them: there’s no problem on doing a smaller PR with the basic <Link> component. If that’s the case, we will mark this Feature Discussion as released and move the remaining user stories to new Feature Discussions to be addressed individually.

Actually, there’s already a Feature Discussion related to the Link component about introducing a “standard setting for prefetching” in Frontity: Auto Prefetch data

Regarding the implementation, the only question was if we should expose the Link component in @frontity/components/link or in libraries.router.Link (to make it a responsibility of the router packages). My opinion is that we should go with the @frontity/components/link approach.

I agree we should go with @frontity/components/link.

I was also thinking about the auto prefetch stuff, but that should definitely be its own PR.

So just so I’m clear, is the current user stories + possible implementation detail enough to get a PR up or do we feel we need to discuss this in more details in terms of what props the component should have?

We don’t have a specific process right now for external contributors because we want to make contributing as easy as possible, at least in this phase of the project. So, as long as they contribute to the code, we take care of the rest.

For the Link component, just go ahead with the PR as it seems like a pretty straight-forward implementation :slightly_smiling_face:

We use the Feature Discussion as the main source of truth for a feature. So feel free to add a post here with your implementation ideas, the things you want to accomplish in the first PR, and then link here the PR (or other relevant links) and update your progress from time to time.

The PR review conversation will happen in the PR and that is fine, but if during the development we feel like we need to make a serious change to the implementation, we try to have that conversation here, in the Feature Discussion.

After the PR is finished, we do a final update here to inform about the Final Implementation. The idea is that anyone reading the Feature Discussion from start to end should have all the information available.


Overtime, once we start receiveing more contributions, we will slowly introduce them to the processes we follow ourselves, which involve:

  • Writing a Feature Discussion to start a conversation.
  • Writing an Implementation Proposal when the work of that Feature is going to be started.
  • Starting a PR with the code, that includes:
    • [ ] Working on the code
    • [ ] Updating the TSDocs of any file modified
    • [ ] Updating the types
    • [ ] Adding or updating Unit tests
    • [ ] Adding or updating End to end tests
    • [ ] Adding or updating TypeScript tests
    • [ ] Updating starter themes
    • [ ] Updating other packages
    • [ ] Adding new issue in the docs repo
    • [ ] Updating the relevant Community discussions
    • [ ] Adding a Changeset (with link to its Feature Discussion if it exists)
  • Add the relevant docs for this feature, done in collaboration with our documentation team.
  • Finally, update the Feature Discussion with the Final Implementation.

Those tasks are already included in our PR template. But for external contributors, we will take care of the remaining tasks when something is missing.


We have a small contribution guide that we need to improve:

And a rather complete implementation proposal template for the most complex features:

This is an example of a FD where we used the IP template:

We don’t want to impose such a large implementation proposal for all the features, some of them will do just fine with a small implementation proposal, like for example this Link component.

Other things in our todo list are:

  • Improve our Contribution guide.
  • Add a Coding Standard guide.
  • Add a Design principles guide.

Probably more. And of course, any feedback is welcomed :slightly_smiling_face:

Thanks @luisherranz.

I’ve got a PR opened for this: https://github.com/frontity/frontity/pull/507

3 Likes

Summary of the current state:

The link is implemented in https://github.com/frontity/frontity/pull/507

User stories implemented

As a theme developer
I want to import a default Link component
so that I don’t have to create it myself

As a final reader
I want to open links in new tabs when I cmd+click
so that I can use the site without hassle

As a Frontity developer
I want to get a warning if my links won’t render a href
so that I can be sure that all my links work

Future features

  • We should support autoprefetching with some sane default strategy according to Auto Prefetch data
  • We could add an an ability to pass state to the page component that is being linked to. E.g:
  <Link link="/posts/some-post-name" state={{ customState: "hello" }}>

Then, whichever component is rendered by the /posts/some-post-name route, gets this state passed as prop.

  • We could provide a variation of the Link component that is route-aware and allows passing optional styles if the current route matches the link of that component. This would be like the NavLink of the react-router.
2 Likes

This has been included in the latest release :tada: : Read the full Release Announcement.

2 Likes

I was wondering, does it make sense to add a feature that fixes local links removing the domain from them?

I’m thinking about something like: link.replace(state.frontity.url, '')

This replace is currently being done by the handlers of @frontity/wp-source but when users need to implement their own handlers, going through every possible link that needs to be fixed in every handler might be a pain.

What do you think?

We are going to do that here: Client routing for internal `content` links

But the post.link fields need to also be correct in the state, or you’d have to be very careful not to use post.link directly anywhere else except inside Link. So I think that you still should transform them in your handlers.

By the way, transforming these links is one of the things that will be easier to do once we introduce the hooks (filters) for the state.

1 Like

Hey everyone.

When I started my project this didnt exist yet so I had it working in the old way. Now I am trying to change it and use the component but for some reason it is not replacing the absolute urls to relative so any link i click on my menus for example, makes a full http request.

I suspect it is not replacing the correctly due to my settings and the url’s placed there and I tried any combination I could think of but couldnt make it work. So here is my component:

import React from "react";
import { connect, styled, useConnect } from "frontity";
import Link from "@frontity/components/link";

const KhLink = ({
  children, 
  ...props
}) => {

  const {state, libraries} = useConnect()
  const StyledAnchor = styled(Link)`
    color: ${(props) => props.color};
    font-weight: ${(props) => props.fontWeight || "bold"};
    position: relative;
    font-family: ${(props) => props.fontFamily};
    font-size: ${(props) => props.fontSize};

    &:visited {
      color: ${(props) => props.color};
    }
  `;

  const onClick = (event) => {
    if (state.shop.sidebarIsOpen) {
      state.shop.sidebarIsOpen = false;
    }
  };


  return (
    <StyledAnchor
      fontFamily={libraries.theme.fonts[props.fontFamily] || libraries.theme.fonts.normal}
      color={props.color || "white"}
      fontSize={props.fontSize || "14px"}
      {...props}
      onClick={onClick}
    >
      {children}
    </StyledAnchor>
  );
};

export default connect(KhLink, { injectProps: false });

It is basically the same as the Link component in mars-theme, just with some extra styling. It is important to note that i tested the same with just the plain Link, component from frontity and it had the same issue. That is why I suspect it has something to do with the settings file.

In my frontity settings i have the following:

const settings = {
  name: "koenighaus-webshop",
  state: {
    frontity: {
      url: "https://neu.koenighaus-heizsysteme.de",
      title: "Koenighaus Heizsysteme",
    },
  },
  packages: [
    {
      name: "@frontity/koenighaus-theme",
    },
    {
      name: "@frontity/wp-source",
      state: {
        source: {
          api: "https://api.koenighaus-heizsysteme.de/wp-json/",
          params: {
            per_page: 9,
          },
          homepage: "/home/",
          postEndpoint: "product",
          postTypes: [
            {
              type: "product",
              endpoint: "product",
              archive: "/webshop",
            },
          ],
          taxonomies: [
            {
              taxonomy: "product_cat",
              endpoint: "product_cat",
              postTypeEndpoint: "product",
            },
          ],
        },
      },
    },
    "@frontity/tiny-router",
    "@frontity/html2react",
    "@frontity/head-tags",
    "woocommerce-shop",
  ],
};

export default settings;

And finally here is a screenshot of the url’s that are in my main menu. The urls are comping as absolute urls returned from the menu endpoint.

Screenshot 2020-09-25 at 17.41.53
This screenshot shows each URL, logged into the console by the link component.

Issue persists both on live and on localhost. Here is a link from the dev/live site: https://koenighaus-webshop.donkoko.vercel.app/. If you hover the links in the main menu you will see that they are still showing the full absolute url to the API.

Hey @ni.bonev! :wave:

I’m really sorry if this wasn’t transparent enough but what you are seeing is the expected behaviour at the moment! The links that are present in the content that you receive from WordPress are not being transformed right now.

We are going to create a processor that replaces the absolute links with “client-aware” links but this feature is not available yet! We have been discussing the implementation in: Client routing for internal `content` links

Luis has pointed this out above in this very thread here

@nicholasio.oliveira has already started some work on the implementation in a pull request: https://github.com/frontity/frontity/pull/520. It would be awesome if you’d like to send more commits to that very same PR to get this feature finished quicker! :sweat_smile: I’d be happy to guide you and you’re welcome to ping me with questions :slight_smile:

1 Like

Hey @mmczaplinski,

thanks for clarifying everything. I thought it was something on my side, so good to know to not spend more time debugging.

I will look at the PR and see if I can help. I was already handling some of the cases myself because it was not working so maybe that helps as well.