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

Offline Mode: Improve pre-publishing sheet #22606

Merged
merged 14 commits into from
Feb 15, 2024
Merged

Conversation

kean
Copy link
Contributor

@kean kean commented Feb 14, 2024

I've made a few fixes and improvements to the pre-publishing sheet and refactor the code to prepare it for the Offline Mode changes.

  • Fix the presentation layout by switching to the native UISheetPresentationController, so the screens no longer jump during transitions
  • Update the date picker to be pushed into the navigation stack and significantly simplify its design and implementation
  • Fix the tint color of the "Publish Date" picker, which was previously using WP blue
  • Slightly tweak the designs of the pre-publishing sheet based on the project spec and the Chris's pcdRpT-2VA-p2
  • Fix the missing "Close" button (it wasn't working even with accessibility enabled). Now, it's always visible to make sure we never miss it during testing.
  • Update PostSettingsViewController to be shown in a form sheet in the editor to align with the Post List view
  • Show the pre-publishing sheet using form presentation on iPad, again, to align with the Post List. It no longer uses popovers. The popovers were not the best solutions considering how complicated the pre-publishing sheet was, and it also wasn't the best solution for the synchronous publishing changes because you don't expect the popover to be non-dismissible. They are usually used for quick interactions.
  • Rename "Publish Now" to "Publish" to align with the web and the project spec

This change can be merged and released independently from the rest of the Offline Mode project changes.

Removing as much of this customization code as possible will go a long way in reducing the risk of this flow breaking in the future, especially considering that there already was a #22012 in Xcode 15-related to the custom presentation controllers.

To test:

The best way to test it is by opening the production version and the new version side by side and verifying that they have the same functionality. All of the changes that were made were cosmetic, and I tried to avoid making any functional changes.

Status Revert

  • Open the pre-publishing sheet
  • Set post status to “Private”
  • Dismiss the screen by either tapping “Cancel” or outside of the sheet
  • Verify that the post status revers to the original status (don't know why it prevents only "private" , but that was the production behavior)

Screenshots

The screenshots and recording of before (on the left) and after (on the right).

1. Sheet presentation

Screen.Recording.2024-02-13.at.7.07.52.PM.mov

2. Dynamic Type

The "Publish" button is now always visible. The separators are hidden unless the content gets obstructed.

Screen.Recording.2024-02-13.at.7.09.39.PM.mov

3. Misc

Screenshot 2024-02-13 at 7 06 52 PM

Note: I don't think the "Now" change is ideal, but it's at least the same location/naming as on the desktop.

Screenshot 2024-02-13 at 7 07 44 PM Screenshot 2024-02-13 at 7 06 44 PM Screenshot 2024-02-13 at 9 10 10 PM

Regression Notes

  1. Potential unintended areas of impact: Pre-publishing sheet
  2. What I did to test those areas of impact (or what existing automated tests I relied on): manual
  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 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.

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.
  • VoiceOver.
  • 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)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@dangermattic
Copy link
Collaborator

dangermattic commented Feb 14, 2024

2 Warnings
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 24.3. This milestone is due in less than 4 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 14, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22606-5d4d3b3
Version24.2
Bundle IDorg.wordpress.alpha
Commit5d4d3b3
App Center BuildWPiOS - One-Offs #8804
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 14, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22606-5d4d3b3
Version24.2
Bundle IDcom.jetpack.alpha
Commit5d4d3b3
App Center Buildjetpack-installable-builds #7834
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean kean added this to the 24.3 milestone Feb 14, 2024
@kean kean requested a review from a team as a code owner February 14, 2024 02:27
@kean kean requested a review from momo-ozawa February 14, 2024 02:30
Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

LGTM w a small nit - the accessibility label for "Connect accounts" and "Not now" in the JP social connections view should be "button", not "text".

Works like a charm otherwise - I went through the testing checklist.

@kean kean force-pushed the task/prepublishing-ui-improvements branch from f304e08 to 9ffaa3a Compare February 15, 2024 00:32
@kean kean removed the request for review from a team February 15, 2024 03:24
@kean kean enabled auto-merge February 15, 2024 12:56
@kean kean merged commit 6f9b8e8 into trunk Feb 15, 2024
18 of 21 checks passed
@kean kean deleted the task/prepublishing-ui-improvements branch February 15, 2024 13:27
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