Clay: badge-error does not exist in the component ClayBadge

Created on 17 Jul 2019  路  5Comments  路  Source: liferay/clay

Hi everyone, I hope everything is ok!

Reading the Lexicon's documentation page for the core badge component it says that there exist 6 different types of badges, see image below:

image

In the Clay's documentation page for the same badge component, but now implemented as a React component, it says that there exist the same 6 badges as specified by the Lexicon's documentation, see image bellow.

image

But if we compare the 2 images we can find an inconsistency in the background color of the badge type error, it was supposed to have a red background but it is actually transparent.

After reviewing the code implementation for this particular component, I found that if we pass displayType="error" prop to this the component , the class badge-error is applied to it as per the code below:

image

After investigating the code, I found that this class (which is a badge variant) is not generated when the ...\packages\clay-css\src\scss\components_badges.scss is compiled to css.

The badges variants are generated by this part of the code (https://github.com/liferay/clay/blob/ee799051689871e07a520385bb8367ba55d5bf27/packages/clay-css/src/scss/components/_badges.scss#L120) which iterates over the $badge-palette. As we can see here the problem is that $badge-palette does not map to any badge-error variant, so the class badge-error is not generated.

image

The closest one to a badge-error class would be the badge-danger class that is generated in the iteration over the $badge-palette.

So I would like to suggest some solutions to this problem:

  • The fastest one is to just map the displayType=error to displayType=danger inside the react component implementation. With this, we follow the Lexicon pattern as well as there is no need to create a new variant to this component.

    • The other one is to create a variant called error in (https://github.com/liferay/clay/blob/ee799051689871e07a520385bb8367ba55d5bf27/packages/clay-css/src/scss/variables/_badges.scss#L110).

Thanks!

documentation

Most helpful comment

hey guys, sorry for that. This was just a typo, the correct one is danger, thanks.

All 5 comments

Good catch @joseigor! I noticed that we have an inconsistency between Lexicon naming of color and our(developer) definition. We follow the Bootstrap naming standard of colors and lexicon have their own. I don't consider an issue but I'm just warning about the inconsistency.

Hey @joseigor, are you planning to send a PR to solve it?

+1 for replacing error with danger :)

Hi, @diegonvs, I am planning to send a PR tonight. I will proceed by replacing the error per danger in the DisplayType to be in accordance with the Bootstrap naming standard. Is this ok for you? Or Do you have a different approach in mind?

hey guys, sorry for that. This was just a typo, the correct one is danger, thanks.

Hi, @diegonvs, I am planning to send a PR tonight. I will proceed by replacing the error per danger in the DisplayType to be in accordance with the Bootstrap naming standard. Is this ok for you? Or Do you have a different approach in mind?

Push it forward @joseigor! :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ethib137 picture ethib137  路  4Comments

sandwichnyc picture sandwichnyc  路  5Comments

bryceosterhaus picture bryceosterhaus  路  4Comments

bicienzu picture bicienzu  路  3Comments

bryceosterhaus picture bryceosterhaus  路  5Comments