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.

Sign up for GitHub

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

Jump to bottom

Feat/design settings improvements #2670

Merged
merged 12 commits into from
Oct 5, 2023
Merged

Feat/design settings improvements #2670

merged 12 commits into from
Oct 5, 2023

Conversation

adekbadek
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

Tweaks the Site Design -> Settings section UI and ads a couple of features.

Before:

image

After:

image

The new "Featured Image Position And Post Template For All Posts" section will allow for bulk-update of featured image and post template. This action is undoable, so there are two levels of warnings added.

How to test the changes in this Pull Request:

  1. Switch to this branch, open Site Design wizard, Settings tab
  2. Modify the "DEFAULT FEATURED IMAGE POSITION FOR NEW POSTS" and "DEFAULT TEMPLATE FOR NEW POSTS" settings, save, and verify they are applied by creating a new post and by checking if the values match in the Customizer (Template Settings -> Post Settings section)
  3. Modify the "FEATURED IMAGE POSITION FOR ALL POSTS" and "TEMPLATE FOR ALL POSTS" settings and observe warnings being displayed below the selections:
image
  1. Click the "Save" button, observe a native browser confirmation dialog kindly ensures if you want to proceed:
image
  1. Click "Cancel" on the confirmation dialog and verify that posts (just pick one) were not updated
  2. Click "Save" again and confirm. Verify posts were updated accordingly

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@adekbadek adekbadek added the [Status] Needs Review The issue or pull request needs to be reviewed label Sep 28, 2023
@adekbadek adekbadek requested a review from a team as a code owner September 28, 2023 09:54
dkoo
dkoo requested changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

I know a lot of pre-launch sites will love these new features! A few ideas and suggestions below:

assets/wizards/site-design/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/views/theme-settings/index.js Outdated
help={ __( 'Set a default template for new posts.', 'newspack' ) }
value={ postTemplateDefault }
options={ [
{ label: __( 'Default', 'newspack' ), value: 'default' },
Copy link
Contributor

Choose a reason for hiding this comment

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

This might warrant an opinion from @laurelfulford, but with the new feature to set a "default" post template for new or existing posts, the name of the "Default" template is confusing to me. It's especially confusing if I change the default template for new posts to something other than "Default", then change the "Template for all posts" to "Default"—part of me thinks doing the latter will change the posts to the default template I had specified in the other setting.

I propose we change the labels for the "Default" template to say something more descriptive like "With sidebar" instead, which would also require renaming the options in the Customizer. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, "Default" is confusing here. I'll update it to "With sidebar" – @laurelfulford let me know if you'd prefer it differently when you're available :)

assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
includes/wizards/class-setup-wizard.php Outdated Show resolved Hide resolved
@adekbadek adekbadek requested a review from dkoo October 2, 2023 15:54
dkoo
dkoo requested changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

The batching works well! Thanks for the optimizations. Just a couple of small nitpicks left.

assets/wizards/site-design/views/theme-settings/index.js Outdated Show resolved Hide resolved
includes/wizards/class-setup-wizard.php Outdated Show resolved Hide resolved
@adekbadek adekbadek requested a review from dkoo October 4, 2023 08:46
@adekbadek adekbadek force-pushed the feat/design-settings-improvements branch from 0fa646d to 1e0d1a9 Compare October 4, 2023 08:47
@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Oct 4, 2023
@adekbadek adekbadek merged commit 37a4656 into master Oct 5, 2023
2 checks passed
@adekbadek adekbadek deleted the feat/design-settings-improvements branch October 5, 2023 08:51
matticbot pushed a commit that referenced this pull request Oct 16, 2023
@semantic-release-bot
chore(release): 2.9.0-alpha.1 [skip ci]
f3886e4 
# [2.9.0-alpha.1](v2.8.3...v2.9.0-alpha.1) (2023-10-16)

### Features

* add Perfmatters to managed plugins ([#2684](#2684)) ([eb657d8](eb657d8))
* **design-wizard:** allow bulk update of featured image, post template ([#2670](#2670)) ([37a4656](37a4656))
* newspack_managed_plugins filter ([#2605](#2605)) ([81ba5f2](81ba5f2))
* **reader-registration:** custom checkbox state for lists ([#2682](#2682)) ([b8ce865](b8ce865))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.9.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@laurelfulford
Copy link
Contributor

I'm sorry I missed the ping on this! 🤦‍♀️ This is a great new feature!

matticbot pushed a commit that referenced this pull request Oct 17, 2023
@semantic-release-bot
chore(release): 2.10.0-alpha.1 [skip ci]
aae5e05 
# [2.10.0-alpha.1](v2.9.0...v2.10.0-alpha.1) (2023-10-17)

### Features

* add Perfmatters to managed plugins ([#2684](#2684)) ([eb657d8](eb657d8))
* **design-wizard:** allow bulk update of featured image, post template ([#2670](#2670)) ([37a4656](37a4656))
* newspack_managed_plugins filter ([#2605](#2605)) ([81ba5f2](81ba5f2))
* **reader-registration:** custom checkbox state for lists ([#2682](#2682)) ([b8ce865](b8ce865))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.10.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Oct 31, 2023
@semantic-release-bot
chore(release): 2.10.0 [skip ci]
0d60353 
# [2.10.0](v2.9.1...v2.10.0) (2023-10-31)

### Features

* add Perfmatters to managed plugins ([#2684](#2684)) ([eb657d8](eb657d8))
* **data-events:** activecampaign connector ([#2663](#2663)) ([377a51f](377a51f))
* **design-wizard:** allow bulk update of featured image, post template ([#2670](#2670)) ([37a4656](37a4656))
* newspack_managed_plugins filter ([#2605](#2605)) ([81ba5f2](81ba5f2))
* **ras:** custom newsletter list selection on registration ([#2706](#2706)) ([57b2871](57b2871))
* **ras:** overlay management for content gate refresh ([#2708](#2708)) ([6767740](6767740))
* **reader-registration:** custom checkbox state for lists ([#2682](#2682)) ([b8ce865](b8ce865))
* remove order-received text ([#2707](#2707)) ([a386524](a386524))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkoo dkoo dkoo approved these changes

Assignees
No one assigned
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adekbadek @matticbot @laurelfulford @dkoo