Skip to content

Sharing: Add bottom margin to sharing buttons section #16811

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
Merged
sixhours merged 1 commit into from
Aug 18, 2020
Merged

Sharing: Add bottom margin to sharing buttons section #16811

sixhours merged 1 commit into from
Aug 18, 2020

Conversation

@sixhours
Copy link
Contributor

@sixhours sixhours commented Aug 11, 2020

Fixes #16619

Changes proposed in this Pull Request:

  • Update sharing CSS to move the bottom margin on the sharing buttons; ul to the outer wrapper, .sd-sharing. This prevents the "Customize buttons" link from running up against the content beneath it.

Before
Screen Shot 2020-08-11 at 4 52 43 PM

After
Logged-in admin on WP.com site
Screen Shot 2020-08-11 at 4 34 39 PM

Not logged in on WP.com
Screen Shot 2020-08-11 at 4 35 05 PM

Local Jetpack, logged in
Screen Shot 2020-08-11 at 4 36 32 PM

Testing instructions:

  • Spin up a new site with this branch
  • Go to Jetpack -> Settings -> Sharing to activate the sharing buttons on posts/pages
  • View a single post on the front end of your site; note spacing around "Customize buttons" link
  • Check different themes to make sure front-end changes have not caused visual regressions or bugs

Proposed changelog entry for your changes:

  • Update spacing around sharing buttons

@sixhours sixhours added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Sharing Post sharing, sharing buttons labels Aug 11, 2020
@sixhours sixhours requested a review from a team August 11, 2020 20:56
@sixhours sixhours self-assigned this Aug 11, 2020
@matticbot
Copy link
Contributor

matticbot commented Aug 11, 2020

Caution: This PR has changes that must be merged to WordPress.com
Hello sixhours! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47980-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 11, 2020

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review
⚠️ The Privacy section is missing for this PR. Please specify whether this PR includes any changes to data or privacy.

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16811

Generated by 🚫 dangerJS against c681ae3

@jeherve jeherve added this to the 8.9 milestone Aug 12, 2020
@jeherve jeherve added the [Status] Needs Review This PR is ready for review. label Aug 12, 2020
jeherve
jeherve approved these changes Aug 12, 2020
View reviewed changes
Copy link
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.

This looks good to me. 👍

@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 Aug 12, 2020
@jeherve
Copy link
Member

jeherve commented Aug 18, 2020

@sixhours Feel free to merge and commit the matching WordPress.com diff whenever you want!

@sixhours
Copy link
Contributor Author

sixhours commented Aug 18, 2020

Feel free to merge and commit the matching WordPress.com diff whenever you want!

Oh I didn't realize I could do that! 😅 Will do!

Is there an order -- ie. should I merge this PR first, then run the WP.com commit/deploy?

@jeherve
Copy link
Member

jeherve commented Aug 18, 2020

Is there an order -- ie. should I merge this PR first, then run the WP.com commit/deploy?

I'd suggest merging in Jetpack first, then committing to WordPress.com, and then coming back here just to post a little comment with the changeset ID of your WordPress.com commit, to close the loop and indicate that this is now on wpcom too.

Thank you!

@sixhours sixhours merged commit 473c4d8 into master Aug 18, 2020
@sixhours sixhours deleted the fix/customize-sharing-button-spacing branch August 18, 2020 15:46
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 18, 2020
@sixhours
Copy link
Contributor Author

sixhours commented Aug 18, 2020

Deployed in r212298-wpcom!

davidlonjon added a commit that referenced this pull request Aug 20, 2020
@davidlonjon
Merge branch 'master' into add/jetpack-lazy-images-package
ff30622 
* master: (23 commits)
  Premium Blocks: set blocks availability (#16898)
  Compat Package: Fix method declaration compatibility (#16900)
  Jetpack Dashboard: More meaningful error notices. (#16883)
  Connection REST API: Unit test for the `remote_authorize` request. (#16879)
  use blog token to request jetpack.updateBlog (#16698)
  Improve Story block media loading (#16663)
  Simplify error notices for broken connections (#16655)
  Use new heartbeat package (#16285)
  wrap-paid-block: remove component. deprecated. (#16895)
  Social Previews: improve preview description handling (#16889)
  Stats module use blog token (#16727)
  Form Block: add a new Consent Field, a new Newsletter setting, and a new newsletter variation (#16808)
  AAG: Backup card, fall back to VP content in case of /rewind API error. (#16867)
  Donations: Fix dependencies (#16892)
  Creative Mail: update option to lowercase (#16861)
  Premium Blocks: Implement the new design (#16611)
  Requests to Stats CSV use the blog token (#16716)
  Update spacing around sharing buttons to avoid no bottom margin below the customize link. (#16811)
  Jetpack SSO: Cleaning up the `requestNonce` API request. (#16830)
  Donations: Update plans when currency changes (#16844)
  ...
jeherve added a commit that referenced this pull request Aug 25, 2020
@jeherve
Changelog: add #16811
c0b7d53
pereirinha pushed a commit that referenced this pull request Sep 10, 2020
@sixhours
[not verified] Update spacing around sharing buttons to avoid no bott…
c3eecae 
…om margin below the customize link. (#16811)
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

@sixhours sixhours

Labels

[Feature] Sharing Post sharing, sharing buttons Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

8.9

Development

Successfully merging this pull request may close these issues.

Sharing: Customize link could use whitespace adjustments.

5 participants

@sixhours @matticbot @jetpackbot @jeherve