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

App Settings Cleanup i #18636

Merged
merged 24 commits into from
May 26, 2022
Merged

App Settings Cleanup i #18636

merged 24 commits into from
May 26, 2022

Conversation

mindgraffiti
Copy link
Contributor

Ref #11925
Ref #18569

Summary

Part 1 of this PR updates some of the UI in App Settings to follow our current color scheme and improves the UX for clearing the Media Cache. This PR does not address the changes to the "Max Image Upload Size" row or the new location option. It is arriving in a separate future PR.

Changes include:

  • Clear Spotlight Index button uses blue brand color and is left aligned
  • Clear Siri Shortcut Suggestions uses blue brand color and is left aligned
  • Media Cache Size UI is updated to show a disclosure arrow and navigates to its own ViewController
  • "Other" section header remains intact because more options are listed in that section compared to when the ticket was created

Screenshots

Before After
Simulator Screen Shot - iPhone 12 Pro - 2022-05-04 at 14 20 06 Simulator Screen Shot - iPhone 12 Pro - 2022-05-04 at 16 04 50

To test:

  1. Install the app on a device, with iOS 12 or higher. Steps 7 and 8 are features found only in iOS 12 and up.
  2. Select "My Site" in the bottom nav baar
  3. Select your profile picture
  4. Choose "App Settings" in the modal view
  5. The "Media Cache Size" row should display the current size of the media cache
  6. Select the row. It should navigate to a new ViewController with the "Media Cache Size" title and size row, as well as a row button to clear the cache. Clear the cache and verify the size reports "Empty"
  7. Navigate back to App Settings
  8. Tap the "Clear Spotlight Index" and wait for a toast message that it was successful
  9. Tap the "Clear Siri Shortcut Suggestions" and wait for a toast message that it was successful

Regression Notes

  1. Potential unintended areas of impact
  • App Settings
  • WPImmuTableRows contains a new row button style
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
    Run all unit tests, which includes:
  • ImmuTableTest
  • MediaFileManagerTests
  1. What automated tests I added (or what prevented me from doing so)
  • None, UI tests don't seem like a valuable use of time

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

Must be left aligned and blue text
This row is a text-only row button, left aligned, using the WordPress blue brand color for the text
Add the table, rows, actions, and other functions to make the Media Cache View Controller work like the new design.
Navigate to the MediaCacheSettingsViewController upon tapping the MediaCacheRow. Remove the mediaClearCacheRow from the AppSettingsViewController. It has moved to the MediaCacheSettingsViewController.
From BrandedButtonRow (it's not a button) to BrandedNavigationRow (it does navigate to a new ViewController)
Improves the user's understanding for this row
@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 17, 2022

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18636-0a021c0 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 18, 2022

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18636-0a021c0 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Copy link
Contributor

@Gio2018 Gio2018 left a comment

Choose a reason for hiding this comment

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

This works as expected @mindgraffiti ! I noticed though that the release notes need to be updated to the next release.
Thank you!

Comment on lines 7 to 8
* [*] App Settings: refreshed the UI with updated colors for Media Cache Size controls, Clear Spot Index row button, and Clear Siri Shortcut Suggestions row button. From destructive (red color) to standard and brand colors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are now targeting 20.0, the release notes should be updated as well.

@mindgraffiti mindgraffiti mentioned this pull request May 24, 2022
4 tasks
@mindgraffiti mindgraffiti requested a review from Gio2018 May 24, 2022 18:04
@Gio2018
Copy link
Contributor

Gio2018 commented May 24, 2022

Hey @mindgraffiti , can you please add the PR reference in the release notes, like the entry above yours? Thank you!

@mindgraffiti
Copy link
Contributor Author

@Gio2018 done! 🙂

@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

Copy link
Contributor

@Gio2018 Gio2018 left a comment

Choose a reason for hiding this comment

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

:shipit:

@mindgraffiti mindgraffiti merged commit 32af0f3 into trunk May 26, 2022
@mindgraffiti mindgraffiti deleted the feature/11925 branch May 26, 2022 19:57
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.

3 participants