Skip to content

Milestone widget: Make sure the widget ID is output#14904

Merged
jeherve merged 1 commit intomasterfrom
fix/milestone-widget-id
Mar 17, 2020
Merged

Milestone widget: Make sure the widget ID is output#14904
jeherve merged 1 commit intomasterfrom
fix/milestone-widget-id

Conversation

@dero
Copy link
Copy Markdown
Contributor

@dero dero commented Mar 6, 2020

The Milestone widget relies on the widget_id being exposed by the before_widget argument passed to register_sidebar in the theme. That is not guaranteed and the widget's live countdown is currently broken in many themes, most prominently in the current twentytwenty.

Changes proposed in this Pull Request:

  • Create a custom ID value derived from widget_id and assign it to every widgets' .milestone-content container.
  • In JS, target the newly assigned ID instead of the unreliable widget_id.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • This fixes a bug in the Milestone widget.

Testing instructions:

  • On master, add the Milestone widget to one of the twentytwenty's Footer sidebars.
  • Set the Milestone date and time to one minute in the future.
  • On the front-end, open the browser's Dev tools and observe XHR requests being sent every second. (Aside: This feels like a DDoS waiting to happen, but I'll take a look into this separately.)
  • Observe the countdown to not update.
  • Switch to this branch.
  • Refresh the page and set the Milestone time and date to one minute in the future again.
  • On the front-end, observe the countdown being updated live.

Proposed changelog entry for your changes:

  • Fix the live countdown feature of the Milestone widget in some themes.

@dero dero added Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets [Status] Needs Review This PR is ready for review. labels Mar 6, 2020
@dero dero requested a review from a team March 6, 2020 14:05
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 6, 2020 •
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: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 9944ec0

@dero dero mentioned this pull request Mar 6, 2020
Merged
@jeherve jeherve added this to the 8.4 milestone Mar 11, 2020
@dero dero mentioned this pull request Mar 17, 2020
Open
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 17, 2020
jeherve
jeherve requested changes Mar 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@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.

Do you think you could rebase that one now that we've merged #14864?

Thanks!

@dero dero force-pushed the fix/milestone-widget-id branch from 88b976b to 9944ec0 Compare March 17, 2020 13:54
@dero
Copy link
Copy Markdown
Contributor Author

dero commented Mar 17, 2020

@jeherve Sure, rebased in 9944ec0.

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 17, 2020
jeherve
jeherve requested changes Mar 17, 2020
View reviewed changes
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 17, 2020
@jeherve jeherve merged commit 656ad9c into master Mar 17, 2020
@jeherve jeherve deleted the fix/milestone-widget-id branch March 17, 2020 16:32
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 17, 2020
jeherve added a commit that referenced this pull request Mar 20, 2020
@jeherve
Changelog: add #14904
8f4790b
jeherve added a commit that referenced this pull request Mar 31, 2020
@jeherve
8.4 Changelog (#15075)
0eb0b35 
* Initial changelog entry

* Changelog: add #14904

* Changelog: add #14910

* Changelog: add #14913

* Changelog: add #14916

* Changelog: add #14922

* Changelog: add #14924

* Changelog: add #14925

* Changelog: add #14928

* Changelog: add #14840

* Changelog: add #14841

* Changelog: add #14842

* Changelog: add #14826

* Changelog: add #14835

* Changelog: add #14859

* Changelog: add #14884

* Changelog: add #14888

* Changelog: add #14817

* Changelog: add #14814

* Changelog: add #14819

* Changelog;: add #14797

* Changelog: add #14798

* Changelog: add #14802

* Changelog: add #13676

* Changelog: add #13744

* Changelog: add #13777

* Changelog: add #14446

* Changelog: add #14739

* Changelog: add #14770

* Changelog: add #14784

* Changelog: add #14897

* Changelog: add #14898

* Changelog: add #14968

* Changelog: add #14985

* Changelog: add #15044

* Changelog: add #15052

* Update to remove Podcast since it remains in Beta

* Changelog: add #14803

* Changelog: add #15028

* Changelog: add #15065

* Changelog:add #14886

* Changelog: add #15118

* Changelog: add #14990

* Changelog: add #14528

* Changelog: add #15120

* Changelog: add #15126

* Changelog: add #15049

* Chanegelog: add #14852

* Changelog: add #15090

* Changelog: add #15138

* Changelog: add #15124

* Changelog:add #15055

* Changelog: add #15017

* Changelog: add #15109

* Changelog: add #15145

* Changelog:add #15096

* Changelog:add #15153

* Changelog: add #15133

* Changelog: add #14960

* Changelog: add #15127

* Changelog: add #15056

* Copy current changelog to changelog archive.

* Clarify changelog description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Extra Sidebar Widgets

Projects

None yet

Milestone

8.4

Development

Successfully merging this pull request may close these issues.

4 participants

@dero @jetpackbot @jeherve @matticbot