-
Notifications
You must be signed in to change notification settings - Fork 32
fix: streams section design #240
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
base: refactor/bff
Are you sure you want to change the base?
Conversation
|
|
||
| setCheckedStatus(newCheckedStatus) | ||
|
|
||
| // sort the grouped streams baseds |
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.
Unable to understand the comment added
| // sort the grouped streams baseds | ||
| if ( | ||
| Object.keys(groupedStreams).length > 0 && | ||
| sortedGroupedNamespaces.length === 0 |
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.
Can you also add a comment on why we are checking for
sortedGroupedNamespaces.length === 0There 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.
to prevent sorting again when user changes streams. will add a comment for same
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.
Cool. Will be helpful for contributors
| </div> | ||
|
|
||
| <div className="flex"> | ||
| <div className={`flex rounded-[4px] ${!loading ? "border" : ""}`}> |
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.
Can we use clsx here
Description
This PR fixes the stream section design issues making it more intuitive and user friendly. Changes include:
Type of change
How Has This Been Tested?
Screenshots or Recordings
Related PR's (If Any):