WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#30677 closed enhancement (fixed)

Customizer: Setting Front page does not work

Reported by: pavelevap Owned by: valendesigns
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch commit
Focuses: javascript Cc:
PR Number:

Description

Steps to reproduce:

  • Visit website (frontend), click some page (or post).
  • Use Customizer link in Toolbar.
  • You can see current page (or post) in Customizer.
  • Try to set up some page as Front page in Customizer settings.
  • There is no live change, only referers page still shown.

Using latest trunk + 2015 theme. When using Customizer from Administration (and not frontend) then everything works.

Attachments (4)

30677.diff (869 bytes) - added by westonruter 5 years ago.
https://github.com/xwp/wordpress-develop/pull/57
30677.2.diff (3.8 KB) - added by valendesigns 4 years ago.
30677.3.diff (4.0 KB) - added by valendesigns 4 years ago.
30677.4.diff (1.1 KB) - added by westonruter 4 years ago.
https://github.com/xwp/wordpress-develop/pull/92

Download all attachments as: .zip

Change History (27)

#1 @SergeyBiryukov
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.1

Confirmed. Doesn't happen in 4.0.1.

#2 @westonruter
5 years ago

Is this not expected behavior?

If I am accessing the Customizer via a page or post, then it this page/post that is shown in the preview as I am taken to the Customizer via a URL like http://src.wordpress-develop.dev/wp-admin/customize.php?url=http%3A%2F%2Fsrc.wordpress-develop.dev%2Fsample-page%2F

Changing the static front page would have no effect when previewing a URL other than home_url() because that setting only applies there. In other words, the Static Front Page section is *contextual* to whether is_front_page()

#3 @westonruter
5 years ago

  • Keywords has-patch added; needs-patch removed

In 30677.diff, make static_front_page section contextual (active) if is_front_page. In this way, the Customizer section will only appear if the front page is being previewed.

Seems too late to add to 4.1, so perhaps should be punted to 4.1.1.

Note that contextual sections were introduced in 4.1 via #29758.

#4 @pavelevap
5 years ago

Yes, my use case was create home page in admin, visiting frontend to see it in action, then browsing some other pages and in the end going to Customizer (from Toolbar) to set up Front page. I did not realize, that there is another page preserved in Customizer and so Front page settings will not be applied. But I am against hidding this settings, because users know that it should be there and they could start search for it and ask what happenned. Maybe some kind of message "You can not see it in action because you are not on Front page" would be better?

#5 @ocean90
5 years ago

  • Milestone changed from 4.1 to Future Release
  • Type changed from defect (bug) to enhancement

I can't see a difference between 4.0 and 4.1 here.
I agree to Weston that this is the expected behaviour and +1 for 30677.diff. Maybe we can put this together with #27630.

#6 @johnbillion
5 years ago

  • Keywords 2nd-opinion added
  • Version changed from trunk to 3.4

Making the front page control contextual sounds like it'll be confusing for users who are customising a page other than their home page and can't find the control.

We should probably just navigate them to the home page in the preview pane if they change the page on front setting.

#7 @westonruter
5 years ago

  • Focuses javascript added
  • Keywords needs-patch added; has-patch 2nd-opinion removed

The front page control could also just have a notice which alerts them when they are not on the homepage.

⚠ This setting can only be previewed on the front page.

And clicking the “front page” link would then navigate the user to the homepage where the setting would be applied if it was changed, or they could then make the change to see it apply. When navigating to the homepage, the notice could slideUp, and when navigating away from the homepage it could slideDown. In terms of how to implement, we can simply enqueue some JS in the Customizer preview which does essentially:

wp.customize.preview.send( 'isFrontPage', <?php echo wp_json_encode( is_front_page() ) ?> );

The static frontpage control would then listen for isFrontPage messages and toggle the visibility according to its boolean value.

#8 @celloexpressions
5 years ago

I'd much rather see the static front page section just be contextual to the front page. On most sites it's only going to be set once, then never changed, so I'd rather not have that UI shown at all when they're not in a context to change it. If they're not on the homepage when they're looking to change that, they should be able to get there easily enough, and I think there's a clear argument for this being expected behavior. I'll also note that users will be customizing the front page more often than other pages since that's where they go first when opening the Customizer from the admin.

This ticket was mentioned in Slack in #core-customize by valendesigns. View the logs.


4 years ago

#10 @celloexpressions
4 years ago

Revisiting this, it looks like there are two potential improvements we could make:

  • Make the static front page section contextual to the front page, potentially adding a notice when not on the front page.
  • Make the preview redirect to the front page/posts page when that setting is changed (my preference).

Let's pick an option or wontfix.

#11 @valendesigns
4 years ago

  • Milestone changed from Future Release to 4.3
  • Owner set to valendesigns
  • Status changed from new to assigned

I think we should refresh the previewer when the option is changed for both page_on_front and page_for_posts. It's fairly simple to do. I just wrote a small snippet of JS that does it from within a theme. Making it work in the Core should be easy and take very little time. I'll work up a patch.

#12 @obenland
4 years ago

@valendesigns any news on that patch? Beta is looming and this ticket looks like it might not make it.

@valendesigns
4 years ago

#13 follow-up: @valendesigns
4 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

@obenland Sorry for the delay! This one took longer than I planned. I had to rethink my approach a bit. Please test 30677.2.diff and critique when you get a chance.

#14 in reply to: ↑ 13 @obenland
4 years ago

Replying to valendesigns:

Please test 30677.2.diff and critique when you get a chance.

I'm probably not the best person for the job, just wanted to make sure there's movement on this ticket.

@valendesigns
4 years ago

#15 @valendesigns
4 years ago

Yeah, no worries. It may need some modifications or be moved around but it does the job. So in theory it should make the cutoff.

#16 @obenland
4 years ago

@westonruter, do you have time to take a look at this this weekend?

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


4 years ago

#18 @westonruter
4 years ago

@valendesigns I took a look at the patch. Maybe I don't understand all that is desired here, but to me it seems like too much is going on?

If the the need is to just change the preview to show the homepage when the page_on_front or page_for_posts settings are changed, then wouldn't it suffice to just do:

api( 'page_on_front', 'page_for_posts', function( setting ) {
        // Change the previewed URL to the homepage when changing the Page on Front.
        setting.bind(function() {
                api.previewer.previewUrl.set( api.settings.url.home );
        });
});

Is there more to it that I'm not accounting for?

#19 follow-up: @valendesigns
4 years ago

@westonruter The code above looks like it only works for the page_on_front option, the page_for_posts option needs to redirect to a different URL. I agree that it's a much simpler approach and worth looking into though. I'll ping you on Slack so we can discuss it.

#20 in reply to: ↑ 19 @westonruter
4 years ago

Replying to valendesigns:

@westonruter The code above looks like it only works for the page_on_front option, the page_for_posts option needs to redirect to a different URL. I agree that it's a much simpler approach and worth looking into though. I'll ping you on Slack so we can discuss it.

OK, take a look at 30677.4.diff. It should update the preview URL as expected when changing the show_on_front, page_for_posts, and page_on_front settings.

If the show_on_front or page_on_front settings get changed, it will update the previewed URL to the homepage if show_on_front is page and page_on_front is not 0.

And then if the page_for_posts gets changed, it will update the previewed URL to wp.customize.settings.url.home + '?page_id=' + value.

Let me know if this does the trick for you.

#21 @valendesigns
4 years ago

It works well, and is a much simpler solution!

#22 @westonruter
4 years ago

  • Keywords commit added; needs-testing removed

I'll commit this.

#23 @westonruter
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 32976:

Customizer: Improve previewing setting changes for show_on_front, page_on_front, and page_for_posts.

When changing the page_on_front setting (with show_on_front), change the previewed URL to be the home URL so that the effect can be seen. When changing page_for_posts, change the previewed URL to be the selected page.

Props valendesigns, westonruter.
Fixes #30677.

Note: See TracTickets for help on using tickets.