-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Dialog: Replies loader #3781
Dialog: Replies loader #3781
Conversation
Build for latest commit 3f13333 is at https://pr3781.build.csb.dev/s/new. |
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.
Looks great, one small comment 😄
packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/index.tsx
Outdated
Show resolved
Hide resolved
); | ||
const Replies = ({ replies, repliesRenderedCallback }) => { | ||
const [isAnimating, setAnimating] = React.useState(true); | ||
const repliesLoaded = !!replies[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.
const repliesLoaded = !!replies[0]; | |
const repliesLoaded = Boolean(replies[0]); |
const SKELETON_HEIGHT = 146; | ||
|
||
React.useEffect(() => { | ||
if (repliesLoaded && !isAnimating) repliesRenderedCallback(); |
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.
if (repliesLoaded && !isAnimating) repliesRenderedCallback(); | |
if (repliesLoaded && !isAnimating) { | |
repliesRenderedCallback(); | |
} |
// export interface ITextProps extends React.HTMLAttributes<HTMLSpanElement> {} | ||
|
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.
// export interface ITextProps extends React.HTMLAttributes<HTMLSpanElement> {} |
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.
Checklist