Merge changes from beta 19.7.0.1 + Release Notes #18411
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,17 @@ | ||
| import UIKit | ||
|
|
||
| class SiteDesignPreviewViewController: TemplatePreviewViewController { | ||
| private let createsSite: Bool | ||
| let completion: SiteDesignStep.SiteDesignSelection | ||
| let siteDesign: RemoteSiteDesign | ||
|
|
||
| init(siteDesign: RemoteSiteDesign, selectedPreviewDevice: PreviewDevice?, createsSite: Bool, onDismissWithDeviceSelected: ((PreviewDevice) -> ())?, completion: @escaping SiteDesignStep.SiteDesignSelection) { | ||
| self.completion = completion | ||
| self.siteDesign = siteDesign | ||
| self.createsSite = createsSite | ||
| super.init(demoURL: siteDesign.demoURL, selectedPreviewDevice: selectedPreviewDevice, onDismissWithDeviceSelected: onDismissWithDeviceSelected) | ||
| delegate = self | ||
| title = TextContent.previewTitle | ||
| } | ||
|
|
||
| required init?(coder: NSCoder) { | ||
|
|
@@ -19,6 +21,17 @@ class SiteDesignPreviewViewController: TemplatePreviewViewController { | |
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| navigationItem.leftBarButtonItem = CollapsableHeaderViewController.closeButton(target: self, action: #selector(closeButtonTapped)) | ||
| setPrimaryActionButtonTitle() | ||
| } | ||
|
|
||
| private func setPrimaryActionButtonTitle() { | ||
| primaryActionButton.setTitle(createsSite ? TextContent.createSiteButton : TextContent.chooseButton, for: .normal) | ||
| } | ||
|
|
||
| private enum TextContent { | ||
| static let previewTitle = NSLocalizedString("Preview", comment: "Title for screen to preview a selected homepage design.") | ||
| static let createSiteButton = NSLocalizedString("Create Site", comment: "Title for the button to progress with creating the site with the selected design.") | ||
| static let chooseButton = NSLocalizedString("Choose", comment: "Title for the button to progress with the selected site homepage design.") | ||
|
Comment on lines
+33
to
+34
There was a problem hiding this comment.
There was a problem hiding this comment. 👋 @AliSoftware thanks for all your information recently about translations. Could I get one bit clarified? If I understand correctly, "Create Site" and "Choose" are keys* for the translated strings. These keys existed prior to this change in WPiOS and had the same meaning [one] [two]. Does this still mean the translations won't work for this view? I ran it locally and set it to a random language (German) and saw it was translated. Thanks! *I realize we may be getting away from the English string being the key soon. There was a problem hiding this comment. Ah! I might have missed something when looking at the diff then, as I was convinced that those were newly introduced strings — as opposed to already existing ones that we just moved around or reused in a different context. If they were already part of the strings that got extracted in the generated There was a problem hiding this comment. Cool! "Create Site" is a "new" string to that view, but it was used (in the same context) in WPiOS prior to this change, so my hope is that the translation for that key is just reused. "Choose" already existed on this view, it was only moved. 👍 |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| We made a small but mighty change to the block editor: the “Add Block” button is more visible when you first open the editor with no blocks selected. We also removed one of three error notifications when a media or audio block fails to upload—it felt like overkill. | ||
|
|
||
| You’ll hear some accessibility tweaks in the VoiceOver experience when you’re rearranging menu items. Instructions and notices are clearer, and the menu hierarchy makes more sense out loud. | ||
|
|
||
| We added a new screen to the site creation process where you can enter your site’s intent. We tested this screen with a small group of users, and we think you’ll like it, too. | ||
|
|
||
| Web previews won’t cut off bottom-of-the-screen notifications when your browser toolbar is visible. | ||
|
|
||
| What’s in a name? Well, if your site has one, you’ll see it in the My Site navigation title. | ||
|
|
||
| When you swipe left on a comment in your Notifications, you won’t see the “Trash” option anymore. Instead you’ll see “Unapprove Comment” and “Approve Comment.” We approve. |
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 that change expected (especially since that does not seem like just a wording update of a single item, but instead removal of an item related to one PR #16106 while adding a different item related to a different PR #18127)?
(cc @twstokes as author of the commit making that change)
And also, do we need to inform our freelance writer (Grace) about that change and new entry, so that she update her Editorialized Release Notes copy (included in this PR) accordingly?
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.
👋 @AliSoftware - this PR contains the backstory of this change.
Summary:
Should I have included the Owl Team on that PR to loop everyone in? Any tips welcome! 🙇
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.
Thanks for the helpful extra context @twstokes !
I think it's always a good idea to ping your Release Manager especially when you change either release notes (like here) or strings (like for the case of the other review comment I left) in a PR that lands in
release/*branch after the code-freeze.Both release notes and strings are frozen during code freeze and we then use those frozen versions to kick off the rest of the process (ask our freelance writer to write nice notes from this frozen draft, send frozen strings to our translation vendor, …), which is why any change to those after code freeze is important to signal to your RM so they can adjust for those last-minute tweaks appropriately 🙂.
That being said, given the additional context you shared here, I think the wording that ended up being used in the Editorialized version of the Release Notes here below is probably OK and matches what ultimately landed?
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.
Thanks @AliSoftware, that makes sense!
I agree!