#25549 closed defect (bug) (fixed)
Twenty Fourteen: implement featured content "tag" mechanism
Reported by: | lancewillett | Owned by: | lancewillett |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Bundled Theme | Keywords: | has-patch |
Focuses: | Cc: |
Description (last modified by )
Because the 3.8 "Featured Content" plugin needs more development help, and probably isn't going to be ready for 3.8, we're going with Plan B.
Goal: add a Featured Content "tag" option to Settings > Reading so authors can simply tag posts with a tag of their choosing to make them appear in the featured content area.
As discussed in October 8, 2013 office hours and after discussions with @photomatt on the direction of the theme and its main goal as a magazine theme.
Attachments (9)
Change History (58)
#7
@
11 years ago
- Keywords has-patch added; needs-patch removed
- Version set to trunk
Patch takes advantage of the implementation in Jetpack, making existing data reusable with Twenty Fourteen.
Additionally, it allows custom solutions to override what's bundled with the theme.
#8
follow-up:
↓ 11
@
11 years ago
Patch looks good in testing. Only minor things I'd want to change in the include file, mostly in code comments. Probably setting default to 6 for Twenty Fourteen for "quantity" setting.
Tested with and without Jetpack activated, works great.
UX note: what do you think about changing the "Tag Name" entry to be a dropdown of tags from the site? Maybe the most used at the top. If not a dropdown, maybe an auto-complete so you don't have to remember or guess the tag.
#10
@
11 years ago
A dropdown of tags could easily get large and ugly. 25549.2.diff uses the existing .suggest
script and ajax callback that's used in post.js.
#11
in reply to:
↑ 8
;
follow-up:
↓ 12
@
11 years ago
Replying to lancewillett:
Patch looks good in testing. Only minor things I'd want to change in the include file, mostly in code comments.
Right, a decision we still have to make is how far we want to deviate from what is in Jetpack. Currently it's only text domains and a strict notice fix (that will make its way upstream). Not bothering with the comments would allow us to update it a lot easier in the future, along the lines of Genericons.
Probably setting default to 6 for Twenty Fourteen for "quantity" setting.
This touches my comment above, too. I specified this in the add_theme_support()
call to be compatible with existing data.
UX note: what do you think about changing the "Tag Name" entry to be a dropdown of tags from the site? Maybe the most used at the top. If not a dropdown, maybe an auto-complete so you don't have to remember or guess the tag.
kovshenin's patch seems fairly trivial and also something we could pass on upstream, I wouldn't mind doing that.
#12
in reply to:
↑ 11
@
11 years ago
Replying to obenland:
Replying to lancewillett:
Right, a decision we still have to make is how far we want to deviate from what is in Jetpack.
Doh, I already made a bunch more changes to make it specific to Twenty Fourteen. Let's discuss more -- but my instinct is to not worry about keeping them in sync.
We can push things upstream without a big problem.
kovshenin's patch seems fairly trivial and also something we could pass on upstream, I wouldn't mind doing that.
Cool, let's test and get it in.
I think it's worth deviating from the Jetpack code and UX for the reason of improving the experience for our exact case -- and because the "FC as a plugin" approach isn't quite ready yet.
#13
follow-up:
↓ 14
@
11 years ago
Also, in inc/featured-content.php
can we use uppercase __CLASS__
for consistency?
#14
in reply to:
↑ 13
;
follow-up:
↓ 15
@
11 years ago
Replying to kovshenin:
Also, in
inc/featured-content.php
can we use uppercase__CLASS__
for consistency?
Yep, patches welcome.
#19
@
11 years ago
Discussing in today's office hours (log) about where to put the UI controls for both the tag selection and the slider / normal layout options.
Everyone agreed it'd be better UX to have both options in the Customizer instead of one in Settings > Reading and the other in the Customizer.
#20
follow-up:
↓ 21
@
11 years ago
I'm looking at the implementation for adding both controls to the customizer (with the slider one just saving the setting for now). Looking through the current implementation, a lot of it will need to be re-written, but the UI will be the same.
Since we're doing it there, does it make more sense to save the options as theme_mods, since they're all theme-specific? Only thing is that they need to be re-set if you switch to a child theme.
#21
in reply to:
↑ 20
;
follow-up:
↓ 26
@
11 years ago
Replying to celloexpressions:
Since we're doing it there, does it make more sense to save the options as theme_mods, since they're all theme-specific? Only thing is that they need to be re-set if you switch to a child theme.
I think theme mods are best, yes.
#22
follow-up:
↓ 23
@
11 years ago
Idea: what if the theme defaults to a tag of "featured" so that if a blog switches to Twenty Fourteen and has posts with that tag, they'll be automatically featured with no changes?
#23
in reply to:
↑ 22
@
11 years ago
Replying to lancewillett:
Idea: what if the theme defaults to a tag of "featured" so that if a blog switches to Twenty Fourteen and has posts with that tag, they'll be automatically featured with no changes?
+1 to "featured" -- it's a reasonable default to pick, used by a number of other themes, and most importantly, having SOME default is going to make it a thousand times more likely that people will find the feature at all.
(disclaimer: made-up number. actual likelihood may vary. do not try this at home.)
@
11 years ago
Rough first pass at moving the featured content settings to the customizer. List of issues forthcoming.
#25
follow-up:
↓ 31
@
11 years ago
25549.customizer.1.diff is a rough first-pass at moving the settings to the customizer. Everything still works to the extent that it did previously, but I ran into several issues:
- The layout and quantity options work as expected, but the tag name doesn't properly update in the customizer (it uses the tag id, which is updated when the tag name option is sanitized), and the hide-tag option doesn't update in the customizer (even when the tag is kept the same). The options do save just fine but you need to reload the customizer page to get them to update in the preview, even after they're live on the site when you save changes. Something's being cached where it shouldn't, I guess, although it didn't appear to be related to the setting of the transient in
featured-content.php
. - We should be running into same issue as generated accent colors (#25580) with the patched approach for setting the featured content tag id. Since we generate the tag id based on the tag name, similarly to the need to generate color variations, I'm starting to think that we should explore improved core support for generated options in the customizer... alternatively we could just do all of the extra filtering as in obenland's patch on #25580.
- I'm not buying the
add_theme_support
approach. If the featured content file is designed to be a "module", including it should be enough to indicate a desire to support it in the theme. A solution for the quantity support (which I ignored for now) should be explored, though. - The new settings are added to
customizer.php
because they logically fit there and the customize_register code can't be put into a function infeatured-content.php
because that would result in nested class declarations (unless we put it outside of the class, in that file). - Speaking of that custom customizer control, perhaps there should be support for the number input type in core? While we're at it, range would be nice and could also utilize min and max.
- When merging my implementation and lancewillett's of the layout settings, I had done it with a radio UI instead of select. That seems more logical to me because there are only two options anyway so it's better if you can see what they are right away (the select sets up an expectation for more than just one alternative, and feels like we're trying to hide the feature); anyone have thoughts on that? Patch also removes a couple of unneeded parameters there (they're the defaults).
ajaxurl
not being defined in the customizer seems like a core bug. The suggest script doesn't appear to work even with the definition of ajaxurl, but it may just be super slow.- I didn't actually implement making a "featured" tag default, just threw it in as the default theme_mod value.
There are obviously a lot of details to work out, but I think this is a much better UI direction for this setting. If anyone has ideas for fixes (or discussions points) for any of the above (particularly the blatant bugs), patch away!
#26
in reply to:
↑ 21
;
follow-up:
↓ 29
@
11 years ago
Replying to lancewillett:
Replying to celloexpressions:
Since we're doing it there, does it make more sense to save the options as theme_mods, since they're all theme-specific? Only thing is that they need to be re-set if you switch to a child theme.
I think theme mods are best, yes.
If we go the theme mod route, we'd essentially make it non-portable between themes. Let's do it via option to make it pluggable and portable, what do you think?
#27
@
11 years ago
Another quick idea I wanted to track, per feedback from Mel Choyce. If sticky posts exist, and no "featured" tag posts and no Featured Content tag has been set, use the sticky posts in the FC area.
This allows sites moving from other themes a quick and easy way to have Featured Content without doing anything -- it's a fallback to our smart defaults.
#28
@
11 years ago
Following our discussion during today's office hours, @obenland is going to finish up this task and make sure we hit the following items:
- Add "featured" tag as default tag. If user doesn't choose a tag, and has this one already, use it.
- Add sticky posts as fallback to having no "featured" tag or any user-chosen tag at all.
- Use an option—not theme_mod—to save the data for better portability and plugin integration.
- Ensure all the UI needed to manage Featured Content exists in the Customizer.
#29
in reply to:
↑ 26
;
follow-up:
↓ 30
@
11 years ago
Replying to obenland:
If we go the theme mod route, we'd essentially make it non-portable between themes. Let's do it via option to make it pluggable and portable, what do you think?
If we want something that's portable between themes, we should be looking into adding a core API rather than just using a option, IMHO a theme shouldn't just use an option because it wants to share it with other themes.
#30
in reply to:
↑ 29
@
11 years ago
Replying to dd32:
If we want something that's portable between themes, we should be looking into adding a core API rather than just using a option, IMHO a theme shouldn't just use an option because it wants to share it with other themes.
Agreed, which is why we're working on a Featured Content plugin (see ticket description above). If we can make 2014's implementation compatible with existing featured content solutions by just using an option rather than theme mods, I don't see a reason not do that though.
#31
in reply to:
↑ 25
@
11 years ago
Replying to celloexpressions:
The suggest script doesn't appear to work even with the definition of ajaxurl, but it may just be super slow.
It's actually the admin CSS. The z-index
is set to 10000, it needs to be a lot higher than this to show though.
#32
@
11 years ago
Latest patch moves FC settings into the customizer. We should decide if we prefer not to use the suggest script or add an inline style to make it work in the customizer.
#33
@
11 years ago
Created #25866 and #25867 to address the core bugs that are problematic for our implementation here (for the suggest script).
See additional discussion in the IRC office hours. We decided to switch the layout-changing UI back to a select dropdown.
#36
@
11 years ago
Things to finish this one up:
- Debug why in some cases the display doesn't refresh, in Customizer and in front-end, after changing the tag option value
- Get the inline CSS working to have the auto-suggest tag list display properl
#37
@
11 years ago
See #25867 for the CSS for the auto-suggest tag (has patches from celloexpressions for both MP6 and old admin CSS.
#38
@
11 years ago
Patch added to #25866 for moving the ajaxurl definition in the customizer into core.
#39
follow-up:
↓ 40
@
11 years ago
Steps to repeat a few bugs with the Featured Content option in the Customizer.
First bug
Posts: set up your test install so that no posts are tagged "featured" and no sticky posts.
- Open Customizer, open Featured Content pane.
- Tag name "featured" is in the input.
- Change the tag to something you do have tagged, with Unit Test sample data "Post Formats" works.
- Save in Customizer (notice here it doesn't refresh the Customizer iframe version of the site -- that's bug 2 below)
- Go to front end, refresh. The posts show up as expected in Featured Content area.
- Open Customizer again, in Featured Content pane clear out the tag and save.
- Go to front end, refresh. Post Formats posts are still in Featured Content area! Should be empty instead.
The only way to clear the front-end view is to make any change in the Customizer and save again. Otherwise the previously selected tag sticks even after clearing the input.
I think it's related to saving the Featured Content input with an empty value.
Second bug
When you change Featured Content tag in the Customizer, the preview iframe doesn't refresh with the correct posts.
#40
in reply to:
↑ 39
@
11 years ago
Replying to lancewillett:
Steps to repeat a few bugs with the Featured Content option in the Customizer.
First bug
Posts: set up your test install so that no posts are tagged "featured" and no sticky posts.
- Open Customizer, open Featured Content pane.
- Tag name "featured" is in the input.
- Change the tag to something you do have tagged, with Unit Test sample data "Post Formats" works.
- Save in Customizer (notice here it doesn't refresh the Customizer iframe version of the site -- that's bug 2 below)
- Go to front end, refresh. The posts show up as expected in Featured Content area.
- Open Customizer again, in Featured Content pane clear out the tag and save.
- Go to front end, refresh. Post Formats posts are still in Featured Content area! Should be empty instead.
Found that when the text field for the Featured Content setting tag_name was emptied, the logic in the settings callback method validate_settings() was not clearing the tag_id. Updated the settings processing logic for the tag_name and tag_id settings in 25549.bug1.patch
#42
@
11 years ago
Thanks rachelbaker and obenland, looking better.
@obenland, I noticed $wp_customize->add_section
for Featured Content section is in both customizer.php and featured-content.php now. Can we remove one of the versions?
#43
@
11 years ago
I can confirm both bugs above are fixed. w00t. This interaction is working really nicely.
#44
@
11 years ago
- Owner set to lancewillett
- Resolution set to fixed
- Status changed from new to closed
In 26079:
#45
follow-up:
↓ 46
@
11 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
It's definitely working a lot better!
There's still some undesired behavior, particularly when you type very slowly, or wait for the suggest script to load. If you start typing a word, then wait for the suggestions, the customizer will sometimes update to the thing that was typed before you click on a suggestion (often resulting in it falling back to sticky posts), then not update once you click on a the suggestion. This seems particularly problematic with multi-word tags; "Post Formats" frequently ends up registering "post" instead.
This would probably be acceptable (though not ideal) behavior, but empty tags are created for each term that runs through the validation function. We need to find a way to prevent partially spelled tags from creating empty tags. If we can make the suggest script run faster, that might help.
#46
in reply to:
↑ 45
@
11 years ago
We could decrease the timeout before the ajax request is made. Or remove the suggest script.
#47
follow-ups:
↓ 48
↓ 49
@
11 years ago
- Resolution set to fixed
- Status changed from reopened to closed
I'd prefer if you didn't re-open tickets. :)
Feel free to open a new ticket to address improving the Ajax interaction of the tag suggestion.
#48
in reply to:
↑ 47
@
11 years ago
Replying to lancewillett:
I'd prefer if you didn't re-open tickets. :)
Feel free to open a new ticket to address improving the Ajax interaction of the tag suggestion.
Sorry, I was actually writing the comment as you committed & closed, so it hit a concurrent edit conflict :)
The UI is fine, the only issue is really that we might get bad results with creating the tag if it doesn't already exist. Not sure that there's a way we could do anything about that, though, so I think we can just leave it as-is for now and see if anyone has issues with it.
Should the implementation in Twenty Fourteen offer the same options as Jetpack, i.e. choosing the maximum number of featured posts and the ability to hide the featured tag from displaying in post meta and tag clouds?