-
Notifications
You must be signed in to change notification settings - Fork 638
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
Prevent crashing on remote cards code #5924
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
thoughts on something like this?
// don't know how to call it (not exported)
// but like a dumber version of the remote card that expects card to be defined already
const _RemoteCard = ({
card,
gutterSize,
carouselRef,
}: {
card: TrimmedCard;
gutterSize: number;
carouselRef: React.RefObject<FlashList<string>> | null;
}) => { ... }
// this is the exported component
export const RemoteCard: React.FC<RemoteCardProps> = ({ id, gutterSize, carouselRef }) => {
const card = remoteCardsStore(state => state.getCard(id));
if (!card || card.dismissed) return null;
return <_RemoteCard card={card} gutterSize={gutterSize} carouselRef={carouselRef} />;
};
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.
That would work too but I already did the work so I'll just merge it as it is
greg-schrammel
approved these changes
Jul 10, 2024
greg-schrammel
pushed a commit
that referenced
this pull request
Jul 19, 2024
* Return undefined from cards store if data is missing * deal with undefined card obj --------- Co-authored-by: Christian Baroni <[email protected]>
greg-schrammel
added a commit
that referenced
this pull request
Jul 22, 2024
* Insufficient Gas * Update bitrise.yml with latest from bitrise.io (#5940) * insufficient native symbol * Prevent crashing on remote cards code (#5924) * Return undefined from cards store if data is missing * deal with undefined card obj --------- Co-authored-by: Christian Baroni <[email protected]> * fix keyboard dismisses option to copy contract address or view on etherscan (#5908) * fix * keyboardShouldPersistTaps * ContextMenuButton * Simplify SwapInputController animated reaction logic for responding to input value changes (#5923) * Remove redundant sliderXPosition update in debounced onTypedNumber * Remove redundant preservedAmount argument in onTypedNumber as the default is true * Pull out resetToZero as its own worklet to reduce redundant code * Remove unnecessary check for amount > 0 now that onTypedNumber is only used for when the amount is > 0 * Replace onTypedNumber with debouncedFetchQuote function * Pull out variables for prev input / output native values for code readability * Calculate native value based on the new amounts and native prices to be more accurate * Simplify resetToZeroValues logic * Convert swap warning to derived value (#5930) * Convert swap warning to derived value * Move out noWarning to NO_WARNING out of derived value * Tidy up swapInputsController repeated logic around `niceIncrementFormatter` (#5931) * Simplify the calls to niceIncrementFormatter Also removes the niceIncrementFormatter call in formattedInputAmount since the input amount has already been set using the niceIncrementFormatter for the cases it cares about. * Remove incrementDecimalPlaces and niceIncrement as shared values as they are only used in the niceIncrementFormatter function * Upgrade react-native-gesture-handler to v2.17.1 (#5925) * Upgrade reanimated to fix crash * Fix type errors from reanimated upgrade * lint * Fix HourglassAnimation * Revert "move nft fetching logic to graphql worker (#5421)" This reverts commit fff25a6. * Bump reanimated to v3.7.1 * Upgrade react-native-gesture-handler to v2.15.0 * Imports * Android * Merge branch '@christian/upgrade-reanimated' into @christian/upgrade-gesture-handler * Fix android build - remove old reanimated shim * Fix RNGH and ZoomableButton Couldn't get these changes to apply via patch-package, so at least for now had to fork RNGH to apply them: rainbow-me/react-native-gesture-handler@0c4b8be * Fix RNGH install * Bump RNGH, bump reanimated patch * Patch GH instead of forking * Fix native Android ZoomableButton Will follow up with some code cleanup but everything should be working. This also removes a bunch of unnecessary wrapper views and layout recalculations in the Android ButtonPressAnimation component. * Delete unneeded @ts-expect-errors * Remove unneeded @ts-expect-error --------- Co-authored-by: Ben Goldberg <[email protected]> Co-authored-by: Ibrahim Taveras <[email protected]> * Update inputMethod to inputAmount anytime the asset changes (#5934) * Update bitrise.yml with latest from bitrise.io (#5940) * :) * Update src/languages/en_US.json Co-authored-by: Matthew Wall <[email protected]> * Update src/__swaps__/screens/Swap/providers/swap-provider.tsx Co-authored-by: Matthew Wall <[email protected]> * Update src/__swaps__/screens/Swap/providers/swap-provider.tsx Co-authored-by: Matthew Wall <[email protected]> * ah😮 * Update src/__swaps__/screens/Swap/providers/swap-provider.tsx Co-authored-by: Matthew Wall <[email protected]> * idk --------- Co-authored-by: Jin <[email protected]> Co-authored-by: Bruno Barbieri <[email protected]> Co-authored-by: Christian Baroni <[email protected]> Co-authored-by: Ben Goldberg <[email protected]> Co-authored-by: Ibrahim Taveras <[email protected]> Co-authored-by: Matthew Wall <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes APP-1640
What changed (plus any additional context for devs)
There was a cast forcing an object to be treated as
TrimmedCard
while this could be undefined hereOnce I removed it, all the types error showed up and dealt with each of those by using optionals.
The first commit from christian also added some safety checks at the store level.
Screen recordings / screenshots
The app works the same way, it won't crash if we unpublish a card that was already stored in zustand.
What to test
Remote cards feature.