Bugfixes #1303
Bugfixes #1303
Conversation
mattgperry
force-pushed
the
fix/opacity-fix
branch
from
ad61001
to
f3bf92b
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 896202d:
|
| as?: keyof ReactHTML | ||
| axis?: "x" | "y" | ||
|
|
||
| // TODO: This would be better typed as V, but that doesn't seem | ||
| // to correctly infer type from values |
There was a problem hiding this comment.
Looking at the code newOrder is going to be ItemData, of which V is just one of the props, is it compatible?
There was a problem hiding this comment.
.map(getValue)
Returns just the V
| onReorder: (newOrder: any[]) => void | ||
| values: V[] |
There was a problem hiding this comment.
Can we add a tsdoc for the prop?
There was a problem hiding this comment.
Also if the values are given is it better to use render callback instead of children?
There was a problem hiding this comment.
I did consider that but I had two thoughts
- I don't want people to have to provide
values, it'll hopefully be deprecated in the medium term but currently its there to allow us to filter out old values - It would only enforce a pattern where it doesn't need to be enforced. In all our examples we loop over
valuesanyway, so we just reduce composability. As a tangible example we currently wrap all the items withAnimatePresencein the tabs demo which you couldn't do with this pattern.
| } | ||
|
|
||
| if (prevProps.isPresent !== isPresent) { | ||
| if (isPresent) { | ||
| projection.promote() | ||
| } else if (!projection.relegate()) { | ||
| safeToRemove?.() | ||
| sync.postRender(() => { | ||
| if (!projection.getStack()?.members.length) { |
There was a problem hiding this comment.
Why do we need to check the stack members? If there are more members in the stack that does not participate in the current transition, shouldn't an unmounting node be safe to remove?
There was a problem hiding this comment.
If there's another stack member taking over from this one, it's in charge of the exit animation and therefore should be in charge of the safe to remove.
There was a problem hiding this comment.
Can we add this as a comment?
| @@ -9,10 +9,9 @@ interface Props { | |||
| } | |||
|
|
|||
| export class VisualElementHandler extends React.Component<Props> { | |||
| componentDidMount() { | |||
There was a problem hiding this comment.
Where do we ensure that the props are up to date initially?
There was a problem hiding this comment.
Thats moved to within VisualElement's mount . This can be forced by the projection node which means all our components will now have the correct props when didUpdate early mounts them
src/projection/shared/stack.ts
Outdated
| this.members.forEach((node) => node.options.onExitComplete?.()) | ||
| this.members.forEach((node) => { | ||
| node.options.onExitComplete?.() | ||
| console.log(node.resumingFrom) |
There was a problem hiding this comment.
This console log needs to be cleanup 🧹
| @@ -203,6 +210,9 @@ export class VisualElementDragControls { | |||
|
|
|||
| private cancel() { | |||
| this.isDragging = false | |||
| if (this.visualElement.projection) { | |||
| this.visualElement.projection.isAnimationBlocked = false | |||
There was a problem hiding this comment.
Do we need to flip the flag in stop as well?
There was a problem hiding this comment.
I think stop calls cancel
mattgperry
force-pushed
the
fix/opacity-fix
branch
from
9bc731a
to
4b48acd
Compare
No description provided.