Add Twentyseventeen Infinite Scroll support

[stoyan0v]

Fixes #5920

Infinite Scroll not working with Twenty Seventeen

Changes proposed in this Pull Request:

Add twentyseventeen.php file in infinite-scroll/themes that includes the correct template parts.
Add styles that match the Twenty Seventeen structure

Testing instructions:

• Activate Twenty Seventeen and Infinite Scroll and verify that the posts are loaded.

[samhotchkiss]

@mendezcode is on maintenance this week, and should be able to take a look!

[laurelfulford]

Thanks for this PR, @stoyan0v!

Overall it looks good! I did a review and found some issues - most are quite small and nitpicky, please don't be daunted by the length:

Functional

• Twenty Seventeen has two footer widget spaces — sidebar-2 and sidebar-3. The infinite scroll setting just need to check for widgets in these spaces to see whether it should be switched to click-to-scroll. There's an example of this in the code for Twenty Thirteen.

• In some themes we also check for other content under the posts — usually a social media menu — to see if infinite scroll should be click-to-scroll (there’s an example of that in the Affinity theme here with the affinity_has_footer_widgets() function). Since Twenty Seventeen's social links only show in the footer area, what do you think about adding a check for it, too?

Code

• Very minor, but there are a couple tabs on empty lines, and one after the code ends in the RTL styles -- specifically on lines 116 and 124 on empty lines, and on 137 after the opening curly bracket.

Visual

• The #infinite-footer never displays for me, and the regular footer is visible briefly when you get to the end of the posts, before new posts are loaded. I'm not sure if this is intentional - if not, there are some example styles to hide the different footers under the "Do it With Style" header here.

• The infinite loader doesn’t appear to be aligned correctly. When there are no sidebar widgets, the posts display in the right column, and the loader appears centred to the page (example). When there are sidebar widgets, the posts appear in the left column, and the loader appears flush to the left (example). It seems like it should be centred under the posts column in both cases.

• Not sure if this is intentional, but some styles don’t appear to be correct for the theme:

  • The fonts Merriweather and Georgia aren't used in the theme
  • The colour blue is used for hover styles in the infinite scroll styles, but the theme is monochromatic. Both can be seen with the 'Older Posts' button, regular and hover, compared to the theme’s button styles, regular and hover.
  • There's a body:not(.custom-background-image) selector, but the theme doesn’t support custom backgrounds (this may be a standard infinite scroll style though - I noticed it's in the other default themes' styles).
  • It's less noticeable, but the infinite footer styles also don't use the theme's colours for links which are #222222 for regular links and #767676 on hover.

• Twenty Seventeen has two custom colour options - a dark colour scheme, and a "custom" scheme determined by a hue slider. For the dark scheme, the 'Older Posts' button colour should change to a lighter shade using the .colors-dark class added to the body. For the custom hue slider scheme, a .colors-custom class is added but the colour that's used is dynamic. It should be possible to inherit the colour most of the time, but some elements may need to stay greyscale. These schemes can be tested under Customizer > Colors.

Some of the theme's styles can be a bit confusing when it comes to the different layouts, and different colour schemes. Please let me know if I can help at all with adjusting those styles, or if you have any questions at all about the above!

[stoyan0v]

Hey @laurelfulford, thank you for the review and sorry for late response.
I've addressed most of the issues, and I think that I will have the final version by the end of the week.

There is something that I would like to discuss.
When we use custom color scheme, I'm unable to style the button with appropriate color.
The problem is that "Infinite Scroll" style the #infinite-handle span element instead the button.
I've tried to style the button, so it can inherit the theme styles, but then infinity.css overwrites it, because it has larger selector #infinite-handle span button ( the theme uses .colors-custom button).

So it seems that the button can't be styled using css only. There is a filter that we can use twentyseventeen_custom_colors_css and we can add our styles, but I am not sure that we need this for only 2 css rows.

Let me know what do you think, and again thank you for your review.

[laurelfulford]

Thanks @stoyan0v!

When we use custom color scheme, I'm unable to style the button with appropriate color.

Ah, too bad! I wasn't sure it would be possible, since the styles are applied to a span rather than the button for infinite scroll. Thanks for giving that a shot!

Using twentyseventeen_custom_colors_css does seem like a bit of overkill for this one case - I'd personally be fine with the button just staying greyscale for the custom colour scheme, like it is with the default colours.

[laurelfulford]

Sorry for the delay @stoyan0v !

Overall, this is looking great - thank you for putting in all this work!

I found a couple small things from the original list, but they’re very nitpicky:

• In the twentyseventeen_has_footer_widgets() function, the social menu ID used doesn’t match what’s used in the theme (I think it might be from the example I provided). It should be has_nav_menu( 'social' ).

• For the infinite scroll button, the colour updates look good! I would just suggest a small change: right now the button text colour is #777 when using the dark scheme, which is a bit lighter than other buttons and has low contrast on hover: https://cloudup.com/caLxRSdTw2C Can this be updated to #222 like the others?

• Lastly, just some small code formatting things in the CSS:

  • Lines 116 and 122 - code is indented with spaces instead of tabs (in both CSS files)
  • Line 146 - space at end of line (in both CSS files)
  • Line 161 - space at end of line (only in twentyseventeen.css)
  • Line 164 - tab on an otherwise empty line (in both CSS files).

That’s it! Just let me know if you have any questions at all about the above!

[stoyan0v]

Hey @laurelfulford, thanks again for your review.

I have addressed these small issues, so I will appreciate if you can have a look again ( maybe for last time 😄 ).

[laurelfulford]

Thanks @stoyan0v! Everything looks good to me - I have no further changes 👍

[zinigor]

Tested on twentyseventeen both with footer widgets and without them - works fine!

[ericswpark]

Hi, I would really like this merged in as soon as possible because it's a PITA to press pages when I scroll to the end. Thanks!

[dereksmart]

hey @stoyan0v! this all looks great and I'd really like to merge this today to make it into the next release.

My only request is to please name space the functions in this file with jetpack_ to eliminate any possibility of redeclaration. Thanks!

(mostly looking at twentyseventeen_has_footer_widgets(), the rest will probably be fine but won't hurt to change for consistency)

[stoyan0v]

Hey @dereksmart. I have namespaced all functions for twentyseventeen.
I didn't add the prefix in the beginning, because the functions in other theme files don't have such( twentysixteen.php, twentyfifteen.php etc ) . So I think that it will be nice to add such prefix to all themes too. I can prepare a separate PR if you agree with me.

[dereksmart]

@stoyan0v thanks, yes that makes sense and a PR would be most welcome :)

Functions are now namespaced.

[jeherve]

This looks good. Merging!

[georgestephanis]

Added this to phabricator for the wpcom merge of it as D4556