-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Feature: Post Settings #24623
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
base: trunk
Are you sure you want to change the base?
Feature: Post Settings #24623
Conversation
Generated by 🚫 Danger |
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 28083 | |
Version | PR #24623 | |
Bundle ID | org.wordpress.alpha | |
Commit | aa03dc6 | |
Installation URL | 4134cfnflde2g |
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 28083 | |
Version | PR #24623 | |
Bundle ID | com.jetpack.alpha | |
Commit | aa03dc6 | |
Installation URL | 38ii3bijo2nn8 |
c583d5a
to
0e2ecc0
Compare
…t is not editable
|
SiteMediaImage(media: image, size: .large) | ||
.loadingStyle(.spinner) | ||
// warning: SiteMediaImage doesn't seem to reload otherwise; might want to change it later | ||
.id(image) |
There was a problem hiding this comment.
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?
let page = try context.existingObject(with: pageID) | ||
page.hierarchyIndex = hierarchyIndex | ||
return page | ||
} |
There was a problem hiding this comment.
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?
import SwiftUI | ||
import DesignSystem | ||
|
||
public struct SectionHeader: View { |
There was a problem hiding this comment.
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") {}
?
|
||
var body: some View { | ||
HStack { | ||
PostSettingsIconView("wpdl-category") |
There was a problem hiding this comment.
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?
|
||
var body: some View { | ||
HStack { | ||
PostSettingsIconView("wpdl-tag") |
There was a problem hiding this comment.
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?
Version |
Project: https://linear.app/a8c/project/form-design-post-settings-c3b06a84aa38/
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 ViewModelPull 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.