Skip to content
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 2 commits into from
Jul 12, 2024

Conversation

brunobar79
Copy link
Member

@brunobar79 brunobar79 commented Jul 9, 2024

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 here
Once 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.

Copy link

linear bot commented Jul 9, 2024

Copy link
Contributor

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} />;
};

Copy link
Member Author

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

@brunobar79 brunobar79 merged commit 0e04051 into develop Jul 12, 2024
6 checks passed
@brunobar79 brunobar79 deleted the @christian/remote-cards-store branch July 12, 2024 20:40
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants