-
Notifications
You must be signed in to change notification settings - Fork 982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding more usecases to lifecycle component #4804
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
hey @JKarlavige whenever you have a chance, can you review this PR please? i believe @nghi-ly and i were asking Jey to clarify the 'gray' color as it's looking washed out when you view the docs in light mode. the hex i used it #f3f4f6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @mirnawong1! Left some feedback, let me know if you have any questions! Especially around the TOC side of it as it might get confusing.
Also, in the task there's a note: Support adding badge to page level (h1)
. Should this be included within this PR, or are we holding off on that for now?
<span className={styles.lifecycle}>{props.status}</span> | ||
); | ||
// Check if props.status is an array or a single value | ||
const statuses = Array.isArray(props.status) ? props.status : [props.status]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great way to ensure statuses
is always an array!
Also wanted to share a shorthand way of achieving the same result using the spread operator:
const statuses = [...props.status];
If props.status
is a string, it will spread it into an array. If props.status
is an array, it will create a clone of it, essentially being the same as the initial array.
thanks so much @JKarlavige ! will review this 🙏 |
…etdbt.com into update-lifecycle
amazing! thank you so much -- @JKarlavige and @nghi-ly this is ready for re-review please loom here: https://www.loom.com/share/b7a0482788674381a70574c9fd0bd503?sid=13b7add0-583b-4c98-a696-23b2d8768373 |
i've also updated the asana task and asked for color verification: https://app.asana.com/0/1200099998847559/1206377128451074/f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! all edits look to work great, nice work getting those adjustments in!
Re: H1s - good call on adding the # H1 header
to pages where this will be needed and adding the lifecycle component to that line. I think that's much cleaner and less lift than trying to support adding the lifecycle to the title from the frontmatter.
Awesome work! 😁
if (!props.status || (Array.isArray(props.status) && props.status.length === 0)) { | ||
return null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
yayyyy! thank you soo much @JKarlavige !! am going to wait for @nghi-ly to come online for next steps wrt to merging 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
re: H1s, just to double-check, will using # h1 title
to override in the MD affect anything adversely like search results, SEO, google search results text, etc?
i'll defer to @JKarlavige on this but after researching, it looks like search engines might give more weight to the original frontmatter title than the overridden |
The The h1 tag provides additional context for search engines as to what the content on the page is related to. Even if the frontmatter |
this pr enhances the existing lifecycle component to add more badge support:
see asana task for more info
*note, still waiting for color and add'l use case before final review.
way to use it
colors
predefined colors are: