Multiple links in InfoBanner

[JanHamara]

Describe the feature you'd like

Hello team, we currently have a need to render multiple links inside <InfoBanner /> for the purposes of document cross-checking automation on DMT validation flow. Example below.

Describe alternatives you've considered

Imo we have 3 options here:

• Refactor <InfoBanner> component to accept an array of links to render below the banner description. { linkUrl: string, linkLabel: string }[]
• Refactor <InfoBanner> component to pass JSX with list of links setup via children prop
• Create a snowflake in DMT for this purpose, if we don't want to integrate such feature into <InfoBanner> in our component library

[div-Leo]

IMO the last two should not be valid options.
If we keep adding snowflakes whenever we need something different from what we have in Wave we'll end up having a duplicated DS locally in our projects and most likely (not always) they won't reflect some possible new changes, in particular now with the rebranding.
The proposal in the second option is a pattern that we generally discourage to avoid again inconsistencies between projects. At the end of the day a DS is to delegate/enforce design decisions to it, if we let every component accept any kind of UI we'd break this intent.

The first option as it is will introduce a breaking change so I'd propose to accept InfoLink | InfoLink[] where type InfoLink = { linkUrl: string, linkLabel: string } and add a conditional check that either renders the link or the array of links.

WDYT?

[JanHamara]

Yeah, totally agree with your point of view, just wanted to offer some alternatives. 👍

Just not sure what you mean in last paragraph with the breaking change, where you would accept the InfoLink | InfoLink [] ? There are two props at the moment, linkText and linkUrl, we should only have single prop, I suppose we agree on this, right?

[martimalek]

I agree with Leo as well, I think he means that having only one prop would be a breaking change (since we would be changing the props that users currently rely on), so we should introduce a new prop (e.g. links) while keeping the linkText and linkUrl as they are now.

We could consider using a discriminating union and the never type to try to keep the API clean, so that we only allow either the linkText + linkUrl or the new links prop (e.g. https://codesandbox.io/s/discriminating-union-props-uhc3bg). Then in the next major we can remove linkText and linkUrl.

[nlopin]

I'm totally against the "pass array of links" approach. Today we need to allow a list of links. Tomorrow, we'll need to pass a subtitle. This will quickly fall into the mess of unrelated props we need to support.

We can keep the current level of granularity of the present component, and provide a more granular level API aside. We can do so by exposing a lower level components (e.g. InfoBanner.Card, InfoBanner.Link, etc) via a separate import.

In Jan's special case, he can recreate the behaviour from the constructing blocks, but the main component will still have maximum visibility:

import { InfoBannerBlocks } from '@freenow/wave/InfoBanner/blocks'

const JanBanner = () => {
  return <InfoBannerBlocks.Card>
    <InfoBannerBlocks.Header>Hello</InfoBannerBlocks.Header/>
    {links.map(link => <InfoBannerBlocks.Link href={link.href}>{link.text}</InfoBannerBlocks.Link>)}
  </InfoBannerBlocks.Card>
}

This way we can keep the style consistent but enable this escape hatch for custom uses

[martimalek]

I hadn't thought of exposing a compound component like Nik suggests, but I really like the idea, we'd be keeping the "default" component simple but at the same time allow our users to compose their own variations without losing our styles and without us having to make changes to support a specific use case.

[div-Leo]

I love the idea of having compound components, Kent C Dodds has a few videos about how he structured his library with this strategy and it's very nice and flexible, up to also exposing the hooks to have better control of these components.

Would we add a new set of components to be exported keeping the default one available to be exported, in order to avoid breaking changes, right?

[JanHamara]

Thanks for the input guys, yeah I like Nik's idea too a lot, I will set up building blocks for the container Card, Header, Description and Links and come back with a link to MR 👍