-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Masonic/Multi Column support [Cleaner Rebase] #99
base: main
Are you sure you want to change the base?
Conversation
1a8d71e
to
d575d54
Compare
@@ -210,7 +211,7 @@ export const DraxList = <T extends unknown>( | |||
} = itemStyles ?? {}; | |||
return ( | |||
<DraxView | |||
style={[itemStyle, { transform: getShiftTransform(originalIndex) }]} | |||
style={[itemStyle, getShiftTransformStyle(originalIndex)]} |
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.
We may want to tweak this to merge transform styles using the function I added when addressing #35
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 like you mean this commit (https://github.com/nuclearpasta/react-native-drax/pull/88/files) I probably need to talk this one out, can't wrap my head around how to use it.
@@ -878,6 +880,9 @@ export interface DraxListProps<TItem> extends Omit<FlatListProps<TItem>, 'render | |||
/** Callback handler for when a list item is moved within the list, reordering the list */ | |||
onItemReorder?: DraxListOnItemReorder<TItem>; | |||
|
|||
/** Callback handler for when list is scrolled */ | |||
onScrollProp?: (scrollEvent: NativeScrollEvent) => void; |
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.
Is this leftover cruft from other branch work?
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.
ah this is another change I added this to our app as well so we could pass in a custom onScroll event. I could put this in another commit if you prefer.
The good news is that ultimately there doesn't seem to be a lot of code for us to review! Still I'd love to step through it and work an example together to understand what's going on here and any potential gaps it has. Thank you! |
If you wanted to try to get together now and crank this out I can be
available.
…On Mon, Jun 14, 2021 at 9:40 PM Joe Lafiosca ***@***.***> wrote:
The good news is that ultimately there doesn't seem to be a lot of code
for us to review! Still I'd love to step through it and work an example
together to understand what's going on here and any potential gaps it has.
Thank you!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#99 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA3QMJNBGYFGPR4BRAJ7QDDTS2VPFANCNFSM46PEJMUA>
.
--
Best Regards,
Andres Garcia
Software Engineer
<http://www.andresthegiant.com>http://www.andresthegiant.com
703.389.1081
|
d575d54
to
a53deb4
Compare
Sorry for the hit and run comments yesterday @afgarcia86 I jumped offline for the day right after that! |
@lafiosca @afgarcia86 Hello, I was wondering what the state of this PR was. This change appears to be what I am needing for my project, but it appears that the PR may have been forgotten. |
Hi @StevenGonzalez. I apologize but Andres and I never managed to coordinate our schedules to review this together, and it fell to the wayside as you noted. Unfortunately, these days I do not have availability to work on significant features like this for the repo. If this PR can suit your needs, I encourage you to use patch-package or a custom fork. |
related: #67