-
Notifications
You must be signed in to change notification settings - Fork 258
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
Implement react-native-reorderable-list #5351
Conversation
Was this tested on iOS? We should definitely do that before merging. |
698fcfb
to
ab84ba9
Compare
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.
Small changes needed, but this looks good.
jestSetup.js
Outdated
jest.mock('react-native-reorderable-list', () => ({ | ||
...jest.requireActual('react-native-reorderable-list'), | ||
useReorderableDrag: () => jest.fn(), | ||
})) |
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.
Missing newline in file.
@@ -113,3 +113,37 @@ export const useSceneScrollHandler = (): SceneScrollHandler => { | |||
|
|||
return handler | |||
} | |||
|
|||
export const useSceneScrollWorkletHandler = () => { |
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.
Please add a comment to this to explain why this is different than the one above.
keyStates[walletId] = { sortIndex: i } | ||
} | ||
account.changeWalletStates(keyStates).catch(error => showError(error)) | ||
}) | ||
|
||
const keyExtractor = (item: string) => item |
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.
I think this needs a useHandler or useCallback as well.
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.
Using a useHandler here somehow broke the feature, not sure why...
Is this ok to leave with just a comment?
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.
Figured it out, needed to use useCallback
ab84ba9
to
4a94fb3
Compare
4a94fb3
to
a23c94d
Compare
06a7961
to
22e402b
Compare
22e402b
to
db60fb2
Compare
db60fb2
to
bc68c9c
Compare
Notes:
May need to go with the new library route.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: