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

Deprecates old in-line notification icons #20068

Conversation

antonis
Copy link
Contributor

@antonis antonis commented Jan 29, 2024

Fixes #20022

Description

This PR removes the notification avatar icons along with the related code and resources

Before After
Screenshot_20240129_190927 Screenshot_20240129_190915

To Test:

  1. Open the Jetpack app and switch to the notifications tab
  2. Verify that the badges are removed from the notifications
  3. Verify that the functionality remains unaffected

Regression Notes

  1. Potential unintended areas of impact

    • Notifications tab
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Manual testing
  3. What automated tests I added (or what prevented me from doing so)

    • Removed the test cases related to the changed code.

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes Testing Checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@dangermattic
Copy link
Collaborator

dangermattic commented Jan 29, 2024

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@antonis antonis marked this pull request as ready for review January 29, 2024 17:14
@antonis antonis added this to the 24.3 milestone Jan 29, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 29, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20068-487f92d
Commit487f92d
Direct Downloadwordpress-prototype-build-pr20068-487f92d.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 29, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20068-487f92d
Commit487f92d
Direct Downloadjetpack-prototype-build-pr20068-487f92d.apk
Note: Google Login is not supported on these builds.

@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR has more than 300 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9e1592f) 40.25% compared to head (487f92d) 54.59%.

Additional details and impacted files
@@                          Coverage Diff                          @@
##           feature/notifications_refresh_p1   #20068       +/-   ##
=====================================================================
+ Coverage                             40.25%   54.59%   +14.34%     
=====================================================================
  Files                                  1455       14     -1441     
  Lines                                 66832      522    -66310     
  Branches                              11034       52    -10982     
=====================================================================
- Hits                                  26903      285    -26618     
+ Misses                                37438      219    -37219     
+ Partials                               2491       18     -2473     

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

Copy link
Contributor

@justtwago justtwago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for removing the bunch of stuff, Antonis. I tested it on my Pixel and noticed no badges. The rest works as expected. 🚀

@antonis
Copy link
Contributor Author

antonis commented Jan 30, 2024

Thank you for reviewing @justtwago 🙇

@antonis antonis merged commit 3e36513 into feature/notifications_refresh_p1 Jan 30, 2024
20 checks passed
@antonis antonis deleted the issue/20022-remove-avatar-notifi-icons branch January 30, 2024 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants