[pull] main from MetaMask:main#336
Merged
Merged
Conversation
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Login screen design fixes: * Replace metamask wordmark animation with static logo. * Change background color to same as in live app * resolve spacing issue in b/w login option switch and unlock button. Jira: https://consensyssoftware.atlassian.net/browse/SL-326, https://consensyssoftware.atlassian.net/browse/SL-323 BugFix: #22770, #22846 <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: Login screen design fixes: * #22770 * #22846 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** https://github.com/user-attachments/assets/485fd6cc-40bd-4ade-a040-409d7168b566 <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/92b33de7-466e-48cd-8701-44ce6caec6f9 https://github.com/user-attachments/assets/13372206-c9de-47ca-a22a-d686b55e494b <img width="304" height="637" alt="Screenshot 2025-11-17 at 5 25 08 PM" src="https://github.com/user-attachments/assets/b0cd96fb-a804-496f-a34d-a9d1e17e8ec8" /> <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Replaces onboarding wordmark animation with a static MetaMask logo, starts fox animation on mount, simplifies background handling, and adjusts spacing/layout with corresponding test updates. > > - **Login UI (`app/components/Views/Login`)** > - Replace `OnboardingAnimation` with static MetaMask wordmark image (`METAMASK_NAME`). > - Start `FoxAnimation` on mount (`setStartFoxAnimation('Start')`). > - Remove theme-based SafeAreaView background override; rely on default theme colors. > - Layout tweaks: top padding, align content to `flex-start`, add margins for logo/field, add `unlockButton` top margin. > - Simplify text field styling (remove `textField` background override) and keep error/help/biometry flows intact. > - **Styles (`styles.ts`)** > - Add `metamaskName` style; adjust `container`, `field`, and CTA spacing; remove unused `ctaWrapper*` and theme-dependent `textField` color. > - **Tests** > - Update snapshots to reflect new layout/logo and removed background override. > - Remove style-specific tests; add checks for static logo presence and 100ms seedless-password check timing. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d0fa9ba. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…2698) # Move Lost Redeems Filtering from Client to API ## Summary This PR refactors the activity filtering logic by moving the responsibility of excluding lost redeems (claim activities with no payout) from client-side filtering to server-side filtering via an API parameter. CHANGELOG entry: null ## Changes ### 1. Added API Parameter (`PolymarketProvider.ts`) - Added `excludeLostRedeems: 'true'` parameter to the activity API call - The API now handles filtering out REDEEM activities with no payout before returning results ```typescript const queryParams = new URLSearchParams({ user: predictAddress, excludeLostRedeems: 'true', // ← New parameter }); ``` ### 2. Updated Documentation (`utils.ts`) - Updated JSDoc comment for `parsePolymarketActivity` to reflect the change - Removed outdated reference to client-side filtering - Added note explaining that the API now handles this filtering ### 3. Removed Obsolete Test (`utils.test.ts`) - Removed test: `'filters out REDEEM activities with no payout (lost positions)'` - This test verified client-side filtering behavior that no longer exists - Kept the test for `'maps REDEEM with payout to claimWinnings entries'` which remains valid ## Why This Change? ### Benefits 1. **Better Performance**: Filtering happens on the API side, reducing data transfer and client-side processing 2. **Simpler Code**: Eliminates unnecessary client-side filtering logic 3. **Single Source of Truth**: The API is responsible for data filtering, reducing potential inconsistencies 4. **Cleaner Architecture**: Separates concerns between data fetching and data presentation ### Previous Implementation The client would receive all REDEEM activities and filter them out locally, which meant: - More data transferred over the network - Unnecessary processing on the client side - The filter logic had to be maintained in the client code ### New Implementation The API excludes lost redeems before sending the response, which means: - Less data transferred - No client-side filtering needed - Centralized filtering logic on the API ## Testing ### Unit Tests - ✅ All 126 tests in `utils.test.ts` pass - ✅ All 5 activity-related tests in `PolymarketProvider.test.ts` pass - ✅ No linter errors ### Test Coverage - Verified that REDEEM activities with payouts are still correctly parsed as `claimWinnings` entries - Confirmed that the API parameter is correctly passed in the query string - Ensured that activity parsing still handles all entry types correctly ## Impact ### User-Facing - ✅ No user-facing changes - ✅ Users will continue to see the same activity list (winning claims only) - ✅ Slightly faster activity loading due to reduced data transfer ### Technical - ✅ Cleaner codebase with less filtering logic - ✅ Better separation of concerns - ✅ More efficient data handling ## Migration Notes - No migration needed - The API already supports the `excludeLostRedeems` parameter - Backward compatible change (API handles missing parameter gracefully) ## Related Files - `app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts` - Added API parameter - `app/components/UI/Predict/providers/polymarket/utils.ts` - Updated documentation - `app/components/UI/Predict/providers/polymarket/utils.test.ts` - Removed obsolete test ## Checklist - [x] Code follows project guidelines - [x] All tests pass - [x] No linter errors - [x] Documentation updated - [x] Commit message follows conventions - [x] No breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Shift lost redeems filtering to the Polymarket activity API via `excludeLostRedeems=true`, remove client-side filtering, and update tests and e2e mocks accordingly. > > - **PolymarketProvider**: > - Add `excludeLostRedeems: 'true'` to activity query in `PolymarketProvider.fetchActivity`. > - **Parsing/Utils**: > - `parsePolymarketActivity` no longer filters zero-payout claim entries; note updated to rely on API filtering. > - **Tests**: > - Remove obsolete unit test for filtering lost `REDEEM` activities in `utils.test.ts`. > - **E2E Mocks**: > - Update activity endpoint docs to `/activity?user&excludeLostRedeems=true`. > - Relax activity URL matchers to allow additional query params in `polymarket-mocks.ts`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 4c4bdee. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
improve carousel design
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry: improve carousel design
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Standardizes carousel card width to 80% of screen, updates snap/index
logic, and simplifies rendering/border styles.
>
> - **TrendingView/SectionCarrousel**:
> - Standardizes sizing: `CONTENT_WIDTH = SCREEN_WIDTH`, `CARD_WIDTH =
0.8 * CONTENT_WIDTH`, fixed `CARD_HEIGHT`.
> - Updates snapping and index calc to use `CARD_WIDTH`
(`snapToInterval`, `handleScroll`).
> - Simplifies item rendering: removes `carouselItemLast` and width
variations; no per-item width branching.
> - Moves border styling from styles to `Box` with
`borderColor=BorderDefault`.
> - Cleans up unused padding/content container styles and related
constants.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
38ac5a3. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: salimtb <salim.toubal@consensys.net>
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
- update confirmation toast icon
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry: null
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Use accent03 colors for the confirmed toast and update tests
accordingly.
>
> - **UI/Toasts**
> - Confirmed toast styling in
`app/components/UI/Predict/hooks/usePredictToasts.tsx`:
> - `iconColor` changed from `theme.colors.success.default` to
`theme.colors.accent03.dark`.
> - `backgroundColor` changed from `theme.colors.accent04.normal` to
`theme.colors.accent03.normal`.
> - Hook dependency array updated to reflect new color refs.
> - **Tests**
> - `usePredictToasts.test.ts` updated to expect confirmed toast
`iconColor` `#13330` (accent03.dark).
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
752140c. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
We seem to be wrongly calculating the order values. The CLOB client code
seems quite different than ours, and they use `roundDown()` when
rounding the price for market orders, but that somehow works for them
because they use the `worstPrice` and their `marketPrice` and we are
using `avgPrice`.
In our case, we want the `takerAmount` to be the exact `shareAmount` we
would get with the dollar amount we are willing to spend and, since we
have that value from `matchBuyOrder`/`matchSellOrder`, we do **not**
need to recalculate it, as it will lose precision.
This not only simplifies the logic, but also removes unnecessary
calculations which were leading to incorrect approximations.
### Note
I want to continue investigating the differences between the CLOB client
logic and ours and see if we should revert to what they do.
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry: null
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: fix order calculations
Scenario: user wants to place a Buy order of $1 in a liquid market
Given share price is $0.11
When user tries to submit the order, we calculate that we get `9.090909090909092` shares
This leads to an `avgSharePrice = $1 / 9.090909090909092 shares = $0.109999999`
Then, if we round this number down, we get a share price of `$0.10`, which is much cheaper than it should be
This will lead to a much higher `takerAmount`, which makes the order fail
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Reworks market order preview to compute maker/taker amounts directly
from matched book sizes, removing avgPrice-based rounding and the unused
roundOrderAmounts helper (and its tests).
>
> - **Predict/Polymarket utils**:
> - Adjust `previewOrder` to derive amounts from order book matches:
> - BUY: `makerAmount = roundDown(size, roundConfig.size)`; `takerAmount
= roundOrderAmount(shareAmount, roundConfig.amount)`.
> - SELL: `makerAmount = roundDown(size, roundConfig.size)`;
`takerAmount = roundOrderAmount(dollarAmount, roundConfig.amount)`.
> - Remove `roundOrderAmounts` and dependency on `avgPrice`.
> - Drop `RoundConfig`-related usage for that helper.
> - **Tests**:
> - Remove `roundOrderAmounts` import and its test suite in
`utils.test.ts`.
> - Keep `previewOrder` tests aligned with the new calculation approach.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
60bd2a3. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )