-
Notifications
You must be signed in to change notification settings - Fork 116
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(model3d): Add flyout for animation clips #1369
feat(model3d): Add flyout for animation clips #1369
Conversation
7da08c9
to
240fc99
Compare
return x.length >= width ? x : new Array(width - x.length + 1).join('0') + x; | ||
}; | ||
|
||
export const formatDurationStr = (duration: number): 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.
Do we have something similar to this in the DurationLabels
component?
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.
Yeah you're right, I just copied this over from the original implementation. I'll move the DurationLabels
implementation into a controls/utils.ts
file and import it here
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.
On further investigation, seems these two serve different functions (although similar). The DurationLabels
method will strip off the hours
if it is zero, but this function wants to always display hh:mm:ss
so unless you have a strong objection, I'll keep this one here
|
||
describe('icon prop', () => { | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
function CustomIcon({ isOpen, ...rest }: object, ref: React.Ref<HTMLDivElement>): JSX.Element { |
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.
Do we need to destructure isOpen
out here?
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.
It's to avoid a console error while running unit tests since isOpen
isn't a valid attribute on a div
@@ -11,9 +11,16 @@ import { decodeKeydown } from '../../../util'; | |||
|
|||
export type Props = React.PropsWithChildren<{ | |||
className?: string; | |||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
icon?: React.ComponentType<any>; |
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.
Was fighting this for a while but couldn't get it to work. Open to suggestions
1408979
to
ce67531
Compare
ce67531
to
e6f56f6
Compare
<AnimationClipsToggle /> | ||
{/* TODO: AnimationClipsFlyout */} | ||
</> | ||
<Settings className="bp-AnimationClipsControl" icon={AnimationClipsToggle}> |
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.
Should this be toggle
, as well?
TODO:
Before:
After: