Eui: EuiBadge should allow for `href`

Created on 6 Feb 2020 · 18Comments · Source: elastic/eui

Currently it only accepts onClick but badges can also link to other pages as well. It should accept href similar to EuiButton.

anyone good first issue

Source

cchaos

Most helpful comment

@RisingGeek There's a good, full description of ExclusiveUnion in the code comments. (And another for the specific case you mention)

The idea is that we needed a way for a component with multiple potential element targets to ensure that the attributes for those discrete elements don't get mixed. For instance, a simple union of HTMLAnchorElement | HTMLButtonElement might allow for anhref attribute to be passed to a <button />, which is invalid and something we want to avoid. ExclusiveUnion is our method of requiring that consumers pass props from _either_ HTMLAnchorElement _or_ HTMLButtonElement, but not a combination of the two.

thompsongl on 21 Feb 2020

❤2

All 18 comments

Hi @aryamanpuri , yes please do! Essentially the component needs to accept an href prop and then output the right html element.

If the consumer passes an href the component should render as link tag <a>
If the consumer passes an onClick the component should render as a button tag <button>

You can essentially look at how EuiButton handles the render:
https://github.com/elastic/eui/blob/f11ea608c8d864e04f2841751ec1f02511ba73ee/src/components/button/button.tsx#L168-L195

And we have two helper services for the TS types you can reuse similar to EuiButton here:

https://github.com/elastic/eui/blob/f11ea608c8d864e04f2841751ec1f02511ba73ee/src/components/button/button.tsx#L79-L96

Be sure to check the whole file as not everything was captured in those permalinks.

Thanks!

cchaos on 19 Feb 2020

may I work on this?

shakti97 on 21 Feb 2020

I checked the code, button component is already sufficient to handle render of anchor or button based on the href prop, so just I need to add the optional href in the badge prop and add it to the button .
@cchaos please correct me if I m going wrong.

shakti97 on 21 Feb 2020

The EuiButton component does correclty handle this situation, but we essentially need to translate that same type of code to EuiBadge to handle it as well since EuiBadge does not extend EuiButton.

cchaos on 21 Feb 2020

Hi @cchaos , I was looking at the EuiButton component and was wondering what does exclusive union does?
export type Props = ExclusiveUnion<EuiButtonPropsForAnchor,EuiButtonPropsForButton>;

RisingGeek on 21 Feb 2020

@RisingGeek There's a good, full description of ExclusiveUnion in the code comments. (And another for the specific case you mention)

thompsongl on 21 Feb 2020

❤2

The EuiButton component does correclty handle this situation, but we essentially need to translate that same type of code to EuiBadge to handle it as well since EuiBadge does not extend EuiButton.

okay will try to do that

shakti97 on 21 Feb 2020

@RisingGeek There's a good, full description of ExclusiveUnion in the code comments. (And another for the specific case you mention)

The idea is that we needed a way for a component with multiple potential element targets to ensure that the attributes for those discrete elements don't get mixed. For instance, a simple union of HTMLAnchorElement | HTMLButtonElement might allow for anhref attribute to be passed to a <button />, which is invalid and something we want to avoid. ExclusiveUnion is our method of requiring that consumers pass props from _either_ HTMLAnchorElement _or_ HTMLButtonElement, but not a combination of the two.