add lifecycle badge component #4528
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
content
There was a problem hiding this comment.
Great work again with this @mirnawong1!! Left one comment requesting one additional improvement to make within the component. This goes into conditional rendering to only return the span tag if props.status exists.
Let me know if you have any questions!!
| export default function Lifecycle(props) { | ||
| return ( | ||
| <span className={styles.lifecycle}>{props.status}</span> | ||
| ); | ||
| } |
There was a problem hiding this comment.
We didn't get to this during our meeting today, but I think there's more more improvement we can make with this. As-is, it will always apply the span tag, even if a user forgets to pass in content into the status prop. This will cause the pill to still appear with no text within it.
What we can do is check if props.status is set before adding the span tag to the page. If not, we should return null here rather than returning the span tag.
Here's React's documentation on how you can do this.
There was a problem hiding this comment.
hey @JKarlavige ! thank you and great flag -- i added the null conditional rendering and if a user doesn't add anything, a small '-' appears at the end but i think it's related to the docusaurus' id assignment.
i also experimented to see if i could pass a props from the frontmatter object (as opposed to the string) and it works too!
| // function Item ({ status }) { | ||
| // if (status) { | ||
| // return null; | ||
| // } | ||
|
|
||
| // return <span className={styles.lifecycle}>{status}</span>; | ||
| // } |
There was a problem hiding this comment.
Just a note to remove this unused code!
| .lifecycle { | ||
| background-color: #047377; /* Teal background to align with dbt Labs' color */ | ||
| color: #fff; /* Light text color for contrast */ | ||
| font-size: 0.4em; /* Slightly smaller text */ |
There was a problem hiding this comment.
Regarding the font-size - em is making the font size relative to the parent element the lifecyle component is a part of, so in this case the header size. One situation where this might cause an issue is if the lifecycle component is used outside of the header.
In this case, .4em would make the font size relative to it's parent. For example, if i add the lifecycle component on it's own line, it's parent element becomes a div, which has a smaller font size set than headers. .4em of the div's font size (1.125rem) would end up small and difficult to read.
One adjustment you can make here is switch from em to rem, which makes the font size relative to the root, which is usually 1rem. If you adjust the font-size to use rem, that size will remain consistent no matter where the component is used. .4rem would be pretty small, but bumping it up a bit should get you close to the font size you had when using em.
Using .9rem for example would say 'this text should always be slightly smaller than the default font size for the site.'
As long as the component is always used with headers, this won't be an issue. But if this is ever used outside of the headers, the font size will likely need to be adjusted.
There was a problem hiding this comment.
thank you @JKarlavige ! ok i've changed it to 0.78rem so it's a bit more relative to the parent. this component should mostly be used for headers so hopefully won't be an issue but it's good to allow flexibility.
|
going to merge this now, thanks so much @JKarlavige !!! |
|
Great work again @mirnawong1!! |
creating a lifecycle badge to enable the docs team and documentarians to use a component allowing them to add a lifecycle badge on specific content.