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

Refactors NoteBlock to Kotlin Part 2 #20718

Merged

Conversation

antonis
Copy link
Contributor

@antonis antonis commented Apr 25, 2024

Description

This is a follow up to #20711


To Test:

  1. Open the Notifications tab
  2. Browse to different types of notifications
  3. Verify no change in the functionality

Regression Notes

  1. Potential unintended areas of impact

    • Notifications
  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)

    • I plan to add tests with a follow up PR

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.

Testing Checklist (strike-out the not-applying and unnecessary ones):

  • WordPress.com sites and self-hosted Jetpack sites.
  • 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)

@antonis antonis added this to the Future milestone Apr 25, 2024
@antonis antonis changed the base branch from trunk to feature/notifications_refresh_p2 April 25, 2024 16:38
@dangermattic
Copy link
Collaborator

dangermattic commented Apr 25, 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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 25, 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
Versionpr20718-ab909b2
Commitab909b2
Direct Downloadjetpack-prototype-build-pr20718-ab909b2.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 25, 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
Versionpr20718-ab909b2
Commitab909b2
Direct Downloadwordpress-prototype-build-pr20718-ab909b2.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Apr 26, 2024

Codecov Report

Attention: Patch coverage is 3.92157% with 294 lines in your changes are missing coverage. Please review.

Project coverage is 40.66%. Comparing base (f476a2f) to head (71c80da).

❗ Current head 71c80da differs from pull request most recent head ab909b2. Consider uploading reports for the commit ab909b2 to get more accurate results

Files Patch % Lines
...id/ui/notifications/blocks/CommentUserNoteBlock.kt 0.00% 124 Missing ⚠️
...s/android/ui/notifications/blocks/UserNoteBlock.kt 0.00% 79 Missing ⚠️
...android/ui/notifications/blocks/HeaderNoteBlock.kt 17.64% 54 Missing and 2 partials ⚠️
...android/ui/notifications/blocks/FooterNoteBlock.kt 0.00% 35 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                          @@
##           feature/notifications_refresh_p2   #20718       +/-   ##
=====================================================================
- Coverage                             82.58%   40.66%   -41.92%     
=====================================================================
  Files                                    15     1485     +1470     
  Lines                                   534    68308    +67774     
  Branches                                 50    11331    +11281     
=====================================================================
+ Hits                                    441    27778    +27337     
- Misses                                   75    38013    +37938     
- Partials                                 18     2517     +2499     

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

@antonis antonis marked this pull request as ready for review April 26, 2024 11:24
@antonis antonis requested a review from jarvislin April 26, 2024 11:25
Copy link

sonarcloud bot commented Apr 26, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@jarvislin jarvislin left a comment

Choose a reason for hiding this comment

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

Tested with different notifications, the result is as expected. Thanks for making the refactors.

@jarvislin jarvislin merged commit 2a1b240 into feature/notifications_refresh_p2 Apr 29, 2024
20 checks passed
@jarvislin jarvislin deleted the task/noteblock-to-kotlin-2 branch April 29, 2024 03:46
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