Skip to content

Conversation

@kean
Copy link
Contributor

@kean kean commented Jun 25, 2025

Project: https://linear.app/a8c/project/form-design-post-settings-c3b06a84aa38/

Screenshot 2025-06-30 at 10 05 20 AM

Note: "Status" field coming later.

Overview

Introduce a new "Post Settings" screen written in SwiftUI. The new implementation is easier to reason about thanks to the use of value types instead of Core Data objects. SwiftUI makes it extremely easy to add new rows or modify existing ones, which I use to make some minor enhancements like showing the author profile picture, and more. In the upcoming weeks, it will make it easy to integrate LLM-based features and nice animations on this screen.

Components:

  • PostSettings – a plain struct to represent the settings edited on this screen (Equatable to make it easy to check if there are any unsaved changes). More details in #24622.
  • PostSettingsViewModel – the settings screen where everything used to be in a ViewController now has a dedicated ViewModel

Pull Requests

Groundwork

Features

Removals

Social Sharing

I've looked into the "Social Sharing" in "Post Settings" and created a prototype to re-implement it in Swift. But during testing, I noticed that section was part of the "Prepublishing" sheet on the web and was missing from "Post Settings". The app shows this section in both "Post Settings" and "Prepublishing" sheet. I checked the analytics, and confirmed there are few people how do that to begin with, so it looks like a good call to simply remove this option for consistency with the web and simplicity, so we don't have to support these two different use-cases.

@kean kean added this to the 26.0 milestone Jun 25, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 25, 2025

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

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 25, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number28146
VersionPR #24623
Bundle IDorg.wordpress.alpha
Commit55b729a
Installation URL4aodfjssdvmlo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 25, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number28146
VersionPR #24623
Bundle IDcom.jetpack.alpha
Commit55b729a
Installation URL5ncu1d50lp0ag
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@kean kean force-pushed the feature/post-settings branch from c583d5a to 0e2ecc0 Compare June 26, 2025 14:11
@kean kean requested a review from crazytonyli June 30, 2025 16:50
@kean kean marked this pull request as ready for review June 30, 2025 16:50
@kean kean requested a review from a team as a code owner June 30, 2025 16:50
SiteMediaImage(media: image, size: .large)
.loadingStyle(.spinner)
// warning: SiteMediaImage doesn't seem to reload otherwise; might want to change it later
.id(image)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line the reason we need the Media.id extension? If there is no other place that needs the Media.id extension, maybe we can remove the extension and use .id(image.managedObjectID) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Identifiable isn't used in id but here, where I do need the media itself.

            .sheet(item: $presentedMedia) { media in
                LightboxView(media: media)
                    .ignoresSafeArea()
            }

let page = try context.existingObject(with: pageID)
page.hierarchyIndex = hierarchyIndex
return page
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be the only time-consuming call. But even when there are thousands of pages (which is of course rare), I don't think the buildPageTree call would be long enough to warrant a loading view. Maybe isLoading can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. We also don't expect it to ever fail, to the failure state is also unnecessary. I simplified it to the following by making pages optional:

 if let pages {
                ParentPageSettingsViewControllerWrapper(
                    pages: pages,
                    selectedPage: currentPage,
                    onSelection: onSelection
                )
                .ignoresSafeArea()
            } else {
                ProgressView()
            }

import SwiftUI
import DesignSystem

public struct SectionHeader: View {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use the default header style in Section("Header text") {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes it a bit easier to read, which we want in a form, and it's more consistent with the WordPress design. I plan to keep using these for forms.


var body: some View {
HStack {
PostSettingsIconView("wpdl-category")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the square.grid.2x2 SF symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are new symbols from WPDS, which I used consistently on all new and existing screens with a few exceptions. The main one is the context menus. Makes it look and feel more WordPress.

We need to find a way to include all the new icons at some point.


var body: some View {
HStack {
PostSettingsIconView("wpdl-tag")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the tag SF Symbol?

@wpmobilebot wpmobilebot modified the milestones: 26.0, 26.1 Jul 3, 2025
@wpmobilebot
Copy link
Contributor

Version 26.0 has now entered code-freeze, so the milestone of this PR has been updated to 26.1.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

@kean kean added this pull request to the merge queue Jul 10, 2025
Merged via the queue into trunk with commit 6af4f4f Jul 10, 2025
27 of 32 checks passed
@kean kean deleted the feature/post-settings branch July 10, 2025 19:31
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.

5 participants