Skip to content

Widgets: concatenate author and Social Icons widget CSS#10280

Merged
jeherve merged 3 commits intomasterfrom
fix/missing-widget-styles
Oct 9, 2018
Merged

Widgets: concatenate author and Social Icons widget CSS#10280
jeherve merged 3 commits intomasterfrom
fix/missing-widget-styles

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Oct 9, 2018

Changes proposed in this Pull Request:

Follow up for #10153

The files were not enqueued anymore as of the PR above, but the CSS
did not get added to the jetpack.css file that is enqueued instead of the singular files.

Testing instructions:

  1. Checkout this branch.
  2. run gulp frontendcss
  3. Check that css/jetpack.css was updated and includes the necessary CSS.
  4. Add the 2 widgets to a site where WP_DEBUG is set to false.
  5. Make sure the widgets look good.

Proposed changelog entry for your changes:

  • Widgets: fix missing CSS for the Authors and the Social Icons Widgets.

Follow up for #10153

The files were not enqueued anymore as of the PR above, but the CSS
did not get added to the jetpack.css file that is enqueued instead of the singular files.
@jeherve jeherve 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 Oct 9, 2018
@jeherve jeherve added this to the 6.6.1 milestone Oct 9, 2018
@jeherve jeherve self-assigned this Oct 9, 2018
@jeherve jeherve requested a review from kraftbj October 9, 2018 15:24
@jetpackbot
Copy link
Copy Markdown
Collaborator

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Oct 9, 2018

To add to the testing instructions, do not test on Twenty Seventeen as that theme's normal styles will hide the issue being addressed. Try another theme, like Twenty Fifteen.

Copy link
Copy Markdown
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

Noting specifically testing with Twenty Seventeen would not duplicate the bug (and how this got through review initially), so tested with Twenty Fifteen per original customer report.

Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

Looks good! CSS is added on fresh build

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Oct 9, 2018
Copy link
Copy Markdown
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.

Merging.

@jeherve jeherve merged commit 084edb9 into master Oct 9, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 9, 2018
@jeherve jeherve deleted the fix/missing-widget-styles branch October 9, 2018 15:56
jeherve added a commit that referenced this pull request Oct 9, 2018
* Widgets: concatenate author and Social Icons widget CSS

Follow up for #10153

The files were not enqueued anymore as of the PR above, but the CSS
did not get added to the jetpack.css file that is enqueued instead of the singular files.

* CSS builder: remove authors file since it was already in the list

* CSS Concatenation: add comments to avoid issues in the future.
@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Oct 9, 2018

Cherry-picked to branch-6.6 in 8820f2f

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants