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

Stories Additional Code Removals #20341

Merged
merged 14 commits into from
Mar 4, 2024

Conversation

antonis
Copy link
Member

@antonis antonis commented Feb 29, 2024

Additional code removal suggestions on top of #20016


To Test:

See #20016


Regression Notes

  1. Potential unintended areas of impact

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

    • N/A
  3. What automated tests I added (or what prevented me from doing so)

    • N/A

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:

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

@dangermattic
Copy link
Collaborator

3 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ 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 Feb 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
Versionpr20341-a6801a9
Commita6801a9
Direct Downloadwordpress-prototype-build-pr20341-a6801a9.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 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
Versionpr20341-a6801a9
Commita6801a9
Direct Downloadjetpack-prototype-build-pr20341-a6801a9.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

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

Project coverage is 40.48%. Comparing base (0ad2125) to head (a6801a9).

Files Patch % Lines
...g/wordpress/android/ui/mysite/menu/MenuActivity.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           issue/stories-removal   #20341   +/-   ##
======================================================
  Coverage                  40.48%   40.48%           
======================================================
  Files                       1458     1458           
  Lines                      66996    66988    -8     
  Branches                   11105    11104    -1     
======================================================
- Hits                       27124    27122    -2     
+ Misses                     37391    37386    -5     
+ Partials                    2481     2480    -1     

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

@antonis antonis mentioned this pull request Feb 29, 2024
12 tasks
drawableLeft = Drawable(R.drawable.ic_pages_white_24dp),
drawableRight = Drawable(R.drawable.ic_pages_white_24dp),
drawableTop = Drawable(R.drawable.ic_pages_white_24dp),
drawableBottom = Drawable(R.drawable.ic_pages_white_24dp),
Copy link
Member Author

Choose a reason for hiding this comment

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

I think any random icon would fit the purposes of the preview

@@ -425,7 +425,7 @@ fun MySiteListItemPreviewWithSecondaryImage() {
MenuItemState.MenuListItem(
primaryIcon = R.drawable.ic_posts_white_24dp,
primaryText = UiString.UiStringText("Plans"),
secondaryIcon = R.drawable.ic_story_icon_24dp,
secondaryIcon = R.drawable.ic_pages_white_24dp,
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Thank you for listing so many pieces of related code that I hadn't noticed. It looks fantastic.

@jarvislin jarvislin merged commit 3f0c500 into issue/stories-removal Mar 4, 2024
19 checks passed
@jarvislin jarvislin deleted the issue/stories-removal-extra branch March 4, 2024 07:13
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.

None yet

4 participants