Skip to content

style: fix notification setting padding styles#23934

Merged
Prithpal-Sooriya merged 3 commits into
mainfrom
cursor/fix-notification-padding-styles-901c
Dec 12, 2025
Merged

style: fix notification setting padding styles#23934
Prithpal-Sooriya merged 3 commits into
mainfrom
cursor/fix-notification-padding-styles-901c

Conversation

@Prithpal-Sooriya

@Prithpal-Sooriya Prithpal-Sooriya commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Description

Update NotificationSettings container padding and line margin to align with app spacing consistency.

Changelog

CHANGELOG entry: style: update notification setting container padding

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MDP-373 https://consensyssoftware.atlassian.net/browse/ASSETS-2120

Manual testing steps

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

Before

Screenshot 2025-12-12 at 11 44 47

After

Screenshot 2025-12-12 at 11 44 09

Pre-merge author checklist

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.

Open in Cursor Open in Web


Note

Tightens NotificationsSettings horizontal padding to 16 and updates divider margin; refreshes snapshot accordingly.

  • Styles (NotificationsSettings):
    • Change container from padding: 24 to paddingLeft/Right: 16 and keep paddingBottom: 48.
    • Update line marginHorizontal from -24 to -16.
  • Tests:
    • Update snapshot to reflect new horizontal padding values.

Written by Cursor Bugbot for commit 6460ac1. This will update automatically on new commits. Configure here.

Reduced horizontal padding from 24px to 16px to align with app spacing
consistency. Also updated line marginHorizontal to match.

Fixes: https://consensyssoftware.atlassian.net/browse/MDP-373
@cursor

cursor Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Prithpal-Sooriya Prithpal-Sooriya changed the title Fix notification padding styles style: fix notification setting padding styles Dec 12, 2025
@Prithpal-Sooriya

Copy link
Copy Markdown
Contributor Author

@cursor check failing unit tests. Update test snapshots.

@cursor

cursor Bot commented Dec 12, 2025

Copy link
Copy Markdown
Contributor

Done! I've updated the failing test snapshot. Here's a summary of the changes:

Changes Made

File Updated: app/components/Views/Settings/NotificationsSettings/__snapshots__/index.test.tsx.snap

The snapshot was updated to reflect the new container padding values:

<RCTScrollView
  style={
    {
      "paddingBottom": 48,
      "paddingLeft": 16,
      "paddingRight": 16,
    }
  }
>

Commit: 74b2644246 - "test: update NotificationsSettings snapshot for new padding"

The test now passes locally and the changes have been pushed to the branch. The CI should re-run and the unit tests should pass now.


Open in Cursor Open in Web

Learn more about Cursor Agents

Updated the snapshot to reflect the new container padding values:
- paddingLeft: 16
- paddingRight: 16
- paddingBottom: 48
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.79%. Comparing base (71b1e6e) to head (74b2644).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #23934       +/-   ##
===========================================
- Coverage   78.89%   68.79%   -10.10%     
===========================================
  Files        4050     4055        +5     
  Lines      106202   106459      +257     
  Branches    21458    21522       +64     
===========================================
- Hits        83789    73241    -10548     
- Misses      16536    27833    +11297     
+ Partials     5877     5385      -492     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The CI render produces an empty View element that was missing from
the snapshot.
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeNotifications
  • Risk Level: low
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

The changes are purely cosmetic CSS styling modifications to the NotificationsSettings component:

  1. Container padding changed from padding: 24 to paddingLeft: 16 and paddingRight: 16
  2. Line marginHorizontal changed from -24 to -16
  3. Snapshot test was updated to match the new styles

These changes:

  • Are visual/layout only with no functional impact
  • Don't modify any component logic, state management, or behavior
  • Don't affect any element IDs or selectors used by E2E tests
  • Were accompanied by the appropriate snapshot test update

While the risk is low, running SmokeNotifications is appropriate to verify that the notification settings UI still renders correctly and is functional after these visual changes. The E2E tests interact with elements by IDs (not positions), so they should pass regardless of padding changes, but it's good practice to verify notification-related flows still work.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@Prithpal-Sooriya Prithpal-Sooriya marked this pull request as ready for review December 12, 2025 11:45
@Prithpal-Sooriya Prithpal-Sooriya requested a review from a team as a code owner December 12, 2025 11:45
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 063b6d3 Dec 12, 2025
99 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the cursor/fix-notification-padding-styles-901c branch December 12, 2025 16:17
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 12, 2025
@metamaskbot metamaskbot added the release-7.62.0 Issue or pull request that will be included in release 7.62.0 label Dec 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.62.0 Issue or pull request that will be included in release 7.62.0 size-XS team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants