Skip to content

Homepage Settings #1#14169

Merged
frosty merged 9 commits intodevelopfrom
feature/homepage-settings
May 27, 2020
Merged

Homepage Settings #1#14169
frosty merged 9 commits intodevelopfrom
feature/homepage-settings

Conversation

@frosty
Copy link
Contributor

@frosty frosty commented May 22, 2020

Refs #14167. This PR adds some of the functionality for the Homepage Settings feature, detailed in the linked issue.

homepage-settings-3

  • Added a new row to Site Settings, allowing the user to navigate to Homepage Settings. Also shows the current value of the 'homepage type' setting
  • Added a new Homepage Settings screen, which allows the user to toggle their homepage type between posts and pages. I've done the toggle optimistically, so it updates the model straight away and reverts the change if the API request fails.
  • If the homepage type is set to Pages, we show rows that display the current selection for the Home and Posts pages, if any have been selected.
  • We fetch the list of all pages on the site when the Homepage Settings screen is displayed so that we can find the titles of any currently selected pages.

What's not included

  • Navigation into the Home and Posts page rows in order to select a different page – that will be coming in a followup PR.
  • PR coming up shortly for the associated WordPressKit changes.

To test

Scenario 1

  • Self-hosted non-Jetpack site – ensure that the Homepage Settings row on the Site Settings screen is not visible

Scenario 2

  • With a Jetpack or WordPress.com site where you have never adjusted this setting, visit the Site Settings screen.
  • Ensure that the Homepage Settings row shows 'Classic Blog'
  • Tap into the Homepage Settings row and ensure the Classic Blog selection is reflected on the Homepage Settings screen
  • Tap Static Homepage and check that the Homepage and Posts Page rows appear, and that they have no detail value
  • Tap back out to Settings and ensure that the Homepage Settings row now reads Static Homepage

Scenario 3

  • On the web, visit the Customizer for your test site, and find the Homepage Settings section there. Change it to Static Homepage, and choose both a Homepage and a Posts Page. Don't forget to save your changes!
  • In the app, refresh your sites list and then navigate into your test site
  • Go to Site Settings, and check that the Homepage Settings row says "Static Homepage"
  • Tap into Homepage Settings, and ensure that both of your selected page titles are shown.
  • Change the site back to Classic Blog
  • Reload the customizer on the web for the site, and check it now shows Classic Blog there too

Scenario 4

  • Build and run on your device, and put it into airplane mode
  • Navigate into Site Settings for your site, and tap Homepage Settings
  • Try changing the homepage type. The operation should fail (the setting should not change), and a notice should be displayed

PR submission checklist:

  • I have considered adding unit tests where possible – I started working on some tests, but I can submit those in a followup PR. I'd like to get these first steps merged for now.
  • 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 – will add to the final PR for this feature

@frosty frosty added this to the 15.0 milestone May 22, 2020
@frosty frosty requested a review from bjtitus May 22, 2020 14:30
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 22, 2020

You can trigger an installable build for these changes by visiting CircleCI here.

@frosty frosty changed the title Homepage Settings 1 Homepage Settings #1 May 22, 2020
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 22, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.


return [
CheckmarkRow(title: HomepageType.posts.title, checked: homepageType == .posts, action: { _ in
self.setHomepageType(.posts)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these self references need to be weak to prevent a retain cycle.

I'm seeing HomepageSettingsViewController sticking around after leaving and it looks like a few copies are also made when visiting repeatedly:

Screen Shot 2020-05-26 at 8 05 08 PM

@frosty frosty requested a review from bjtitus May 27, 2020 11:34
@frosty
Copy link
Contributor Author

frosty commented May 27, 2020

Thanks for catching this! Changes pushed, and I don't see any HomepageSettingsViewControllers hanging around any more.

Copy link
Contributor

@bjtitus bjtitus left a comment

Choose a reason for hiding this comment

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

✅ Scenario 1
✅ Scenario 2
✅ Scenario 3
✅ Scenario 4

Looks good to me! :shipit:

@frosty frosty merged commit 9b61f57 into develop May 27, 2020
@frosty frosty deleted the feature/homepage-settings branch June 6, 2020 06:58
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