-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: implement direct children list view #341
feat: implement direct children list view #341
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…st-instead-of-map
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.
First pass looks promising. Great work! I noticed some weirdness in the error display:
weird_starmap.mov
#342 needs merging and rebase this ☝🏽 and it should be green. |
…st-instead-of-map
* | ||
* @returns {string} | ||
*/ | ||
export const getDescription = (issueBodyText: string): string => { |
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.
Function getDescription
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
@whizzzkid ready for review now. sorry for the delay. |
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.
Thanks for fixing the build, I am still seeing this: #341 (review)
did you get an opportunity to fix this?
@whizzzkid What's the issue? that issues show up that then go away? I assume this is because we have partial data and then more full data (i.e. no ETA, then ETA) so it adds/removes appropriately. I think it's fine as is, but I can do skeleton loading on errorNotificationDisplay if globalLoading is still true; would that satisfy issues you have with it? |
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.
…st-instead-of-map
fixes #120
demo: https://starmaps-git-120-feature-request-create-a-vi-1e48b6-ipfs-ignite.vercel.app/roadmap/github.com/ipfs/ipfs-gui/issues/106#list