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

Twenty Twenty: Ensure full compatibility with upcoming default theme #13516

Merged
merged 18 commits into from
Oct 28, 2019
Merged

Twenty Twenty: Ensure full compatibility with upcoming default theme #13516

merged 18 commits into from
Oct 28, 2019

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Sep 23, 2019 •
edited
Loading

Fixes #13511, #13718

Changes proposed in this Pull Request:

  • This PR adds a set of compat files to make sure Jetpack is fully compatible with the future WordPress default theme.

This will then be ported to WordPress.com in D33027-code.

Still to do / check

Testing instructions:

  • Enable the Twenty Twenty Theme on your site. https://github.com/wordpress/twentytwenty
    (You'll want to clone the repo, or download a zip, unzip it, remove -master from the unzipped directory, rezip it, and upload it to your site).
  • Make sure it behaves properly, with all Jetpack features on and when using lots of widgets

Proposed changelog entry for your changes:

  • Twenty Twenty: Ensure full compatibility with upcoming default theme

@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Infinite Scroll [Feature] Theme Tools [Status] In Progress [Pri] High labels Sep 23, 2019
@jeherve jeherve added this to the 7.9 milestone Sep 23, 2019
@jeherve jeherve requested a review from a team as a code owner September 23, 2019 15:28
@jeherve jeherve self-assigned this Sep 23, 2019
@jetpackbot
Copy link

jetpackbot commented Sep 23, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against 2a023e4

@jeherve jeherve mentioned this pull request Sep 23, 2019
Closed
6 tasks
@jeherve jeherve force-pushed the add/twentytwenty-compat branch 2 times, most recently from 3720501 to d821c60 Compare October 2, 2019 13:24
@MichaelArestad
Copy link
Contributor

Is there anything I can do to help with this?

@joanrho
Copy link
Contributor

joanrho commented Oct 10, 2019 •
edited
Loading

@jeherve after chatting with @MichaelArestad about this earlier today, we recommend having the extra Jetpack widgets toggles auto-activated on connection. The settings for them are hidden in /wp-admin/admin.php?page=jetpack#/writing, and most users would not know they need to switch on both toggles in this location in order to see the Jetpack widgets appear on the Widgets page. Let's continue the discussion on a separate PR so this one isn't blocked by that.

@jeherve
Copy link
Member Author

jeherve commented Oct 11, 2019

@joanrho That is indeed probably part of a larger discussion we can have, since those changes were made not that long ago as part of the "Refining Module Prioritization" project.

Internal references:

  • p1HpG7-6vh-p2
  • p1HpG7-6xr-p2

@joanrho
Copy link
Contributor

joanrho commented Oct 17, 2019

Hey, @jeherve! Thanks your help in figuring out why my changes weren't being reflected locally—selecting theme "Twenty Twenty" instead of "Twenty Twenty-master" resolved it. Moved over my style edits from #13719 over to this compat file instead and it looks like it's working well. Can you give it a spin and see if all looks alright on your end? We can then close that other PR since this will take care of it.

jeherve
jeherve commented Oct 18, 2019
View reviewed changes
Copy link
Member Author

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Quick Tabs > Spaces change

modules/theme-tools/compat/twentytwenty.css Outdated Show resolved Hide resolved
modules/theme-tools/compat/twentytwenty.css Outdated Show resolved Hide resolved
@jeherve jeherve mentioned this pull request Oct 21, 2019
Closed
MichaelArestad
MichaelArestad reviewed Oct 23, 2019
View reviewed changes
Copy link
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

I just have some minor concerns. None of them is a blocker. I'm curious to see what you think. Also, the code freeze is upon us. What can we do to help out further?

modules/theme-tools/compat/twentytwenty.css Show resolved Hide resolved
modules/theme-tools/compat/twentytwenty.css Show resolved Hide resolved
modules/theme-tools/compat/twentytwenty.css Outdated Show resolved Hide resolved
@jeherve
Copy link
Member Author

jeherve commented Oct 25, 2019

The infinite scroll button background color isn't showing up for me. It does look correct in the customizer, though.

You're right, my bad. I had not accounted for defaults. This should be fixed in 6821c6c

Having trouble testing sharing buttons and likes (mixed content or content blocker maybe?).

This is odd. Do you not see them at all when the modules are active? How about in a different browser?

@MichaelArestad
Copy link
Contributor

This is odd. Do you not see them at all when the modules are active? How about in a different browser?

I tried different browsers and even a JN site. The odd thing is they show up for me on other sites. I finally goot them to show up. They look good as is.

The last thing for me to get fixed up I think is the related posts styling.

@MichaelArestad
Copy link
Contributor

Fixed up RP (non-block) styles and headings to look like this:

image

@MichaelArestad
Copy link
Contributor

I think as far as theme compat, we're good to go. I don't want to do too much more with this PR. Anything further we can follow up with.

@MichaelArestad MichaelArestad mentioned this pull request Oct 25, 2019
Merged
@jeherve
Copy link
Member Author

jeherve commented Oct 25, 2019

I'm good with this as well. Let's get a review and get this in!

Thanks for all the help!

@jeherve jeherve added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Oct 25, 2019
@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Oct 26, 2019
@jeherve jeherve merged commit 5e411ee into master Oct 28, 2019
@jeherve jeherve deleted the add/twentytwenty-compat branch October 28, 2019 11:47
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 28, 2019
@jeherve jeherve added the [Status] Needs Testing We need to add this change to the testing call for this month's release label Oct 28, 2019
@jeherve jeherve mentioned this pull request Oct 28, 2019
Closed
@MichaelArestad
Copy link
Contributor

Thanks @kraftbj !

@jeherve jeherve added [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Oct 28, 2019
jeherve added a commit that referenced this pull request Oct 28, 2019
@jeherve
Changelog: add #13516
567f47f
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
@jeherve jeherve mentioned this pull request Nov 13, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelArestad MichaelArestad MichaelArestad left review comments

@kraftbj kraftbj kraftbj approved these changes

Assignees

@jeherve jeherve

Labels
[Feature] Infinite Scroll [Feature] Theme Tools [Pri] High Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
7.9
Development

Successfully merging this pull request may close these issues.

Theme Tools / Infinite Scroll: add support for Twenty Twenty
6 participants
@jeherve @jetpackbot @MichaelArestad @joanrho @kraftbj @matticbot