DOP-4028: Fix margin for links with svgs

[anabellabuckvar]

Stories/Links:

DOP-4028

Current Behavior:

The spacing for the caret dropdown icons on the side nav is slightly out of line
[staging] (https://preview-mongodbrayanglerstg.gatsbyjs.io/cloud-docs/gc-test-img/)

Staging Links:

Docs-landing

Notes:

As part of DOP-3989, some css was added to add a margin for svgs nested in links. It seems that it was necessary for correct spacing because the last link in the products list didn't space the arrow correctly due to how the link was being generated. However, the margin fix ended up caused the spacing to change slightly for the caret dropdown icons on the side nav.

In fixing this, I noticed that the method of displaying arrows in Links.js was different for LGLinks, which used a LG prop, than for GatsbyLinks, which used ArrowRightIcon. This was rendering them with slightly different types of arrows as well as margins between link text and the arrows. For consistency, I changed both to use ArrowRightIcon instead

[rayangler]

It's not typical to use a css variable as an element. Did you mean to use a styled component?

As-is, I also don't see the left margin being applied (due to the use of the css variable). I'm curious if this means that we don't even need the css and some whitespace is all we need?

[anabellabuckvar]

implemented these changes

[rayangler]

Is it possible to leave the arrowAppearance prop as-is? I think it might be preferable to use LG's built-in prop for this instance

[caesarbell]

I agree it would have been preferable, but I think the issue could become both arrows rendering or some rendering conflict in general.

[caesarbell]

That might be the trade-off or I wonder if we could leverage the custom className and apply the className value in the ProductItem.js file

<StyledArticle>
        <SectionHeader customStyles={customStyleHeader}>
          {argument.map((child, i) => (
            <ComponentFactory nodeData={child} key={i} />
          ))}
        </SectionHeader>
        {children.map((child, i) => (
          <ComponentFactory nodeData={child} key={i} showLinkArrow={true} className='some-custom-className' />
        ))}
      </StyledArticle>

Then we apply the style using that custom class name. This way it doesn't have an effect on the other links.

[caesarbell]

For more clarity.

ProductItem

const customStylesForArrows = css` // choose a better name
  > svg {
    margin-left: 3px;
  }
`;

// then pass it down as a prop
{children.map((child, i) => (
  <ComponentFactory nodeData={child} key={i} showLinkArrow={true} className={cx(customStylesForArrows)} />
))}

Then in RefRole we can pass that down to the Link in question

 const stylingClass = cardRef ? cardRefStyling : '';
  if (url) {
    return (
      <Link className={cx(stylingClass, className)} to={url} showLinkArrow={showLinkArrow}>
        {children.map((node, i) => (
          <ComponentFactory key={i} nodeData={node} />
        ))}
      </Link>
    );
  }

This should only allow the style to be applied from the ProductItem component as the style class is defined there.

[caesarbell]

@rayangler thoughts? ☝️

[rayangler]

I think both this and Anabella's current solution are reasonable.

Ideally, we would just use the prop from LG's Link component, but unfortunately, since we use both LG Link and Gatsby Link, it makes things a little annoying and/or inconsistent. The benefit of the current solution is that it enforces consistency in both styling (even if the difference is subtle) and implementation while keeping the styling all within the component (big win in terms of readability and traceability).

Since the intention of this change is to prevent a cascading shift in styling for unintended elements (like the side nav items), I think the current solution is fine for now. In the future, we should revisit if it's possible and/or desirable to render a Gatsby Link as a LG Link so that we can much more cleanly use the same shared prop.

[rayangler]

Can you also please update the staging link @anabellabuckvar ? I think the previous changes (incorrect link) are still present.

[caesarbell]

I left some suggestions.

[caesarbell]

I agree it would have been preferable, but I think the issue could become both arrows rendering or some rendering conflict in general.

[caesarbell]

That might be the trade-off or I wonder if we could leverage the custom className and apply the className value in the ProductItem.js file

<StyledArticle>
        <SectionHeader customStyles={customStyleHeader}>
          {argument.map((child, i) => (
            <ComponentFactory nodeData={child} key={i} />
          ))}
        </SectionHeader>
        {children.map((child, i) => (
          <ComponentFactory nodeData={child} key={i} showLinkArrow={true} className='some-custom-className' />
        ))}
      </StyledArticle>

Then we apply the style using that custom class name. This way it doesn't have an effect on the other links.

[rayangler]

LGTM, as long as the console.log is removed

[rayangler]

Can we remove this?

[caesarbell]

@anabellabuckvar outside of our convo, if it doesn't work this solution is okay as well. I will mark approved to not block or hold you up.

[caesarbell]

Another option could be adding the style changes in the RefRole.js if this spacing needs to be more globally acceptable.