8000 Broken email button fixes by benlk · Pull Request #998 · WPBuddy/largo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Broken email button fixes #998

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
merged 6 commits into from
Dec 10, 2015
Merged

Broken email button fixes #998

merged 6 commits into from
Dec 10, 2015

Conversation

benlk
Copy link
Collaborator
@benlk benlk commented Dec 2, 2015

Changes

  • The onClick function for the custom social buttons javascript now works on child elements of .custom-share-button as well as .custom-share-button
  • The custom social buttons javascript is now accessible at window.largo_sharer
  • A +0.2em padding-right fix for Add right-hand padding to horizontal social toolbar #996.

Why

The reason the email button doesn't work on the floating social buttons is because the social sharing button's sharer.init() (that binds the onclick handler for the email popup) is only run on page load, when the floating social buttons don't exist yet. Does it make sense to make the sharer object a window variable so that the floating social button creation function can rerun sharer.init?

When the popup appears for email, it's because the onClick handler is only called when the span.custom-share-button is clicked, not any of the child elements of that element. When it doesn't pop up, it's because an element within the span was clicked. 😞

For #996, the padding fix is to counter the 0.2em margin-left on the icon in the button.

For WE-18 and RNS-41 and #961.

@benlk benlk added this to the hotfix milestone Dec 3, 2015
@benlk benlk added the priority: high Either blocks work on a priority-normal task or a solution here informs other work. label Dec 3, 2015
@benlk benlk closed this Dec 3, 2015
@benlk
Copy link
Collaborator Author
benlk commented Dec 3, 2015

Refiling against master as a hotfix.

@benlk
Copy link
Collaborator Author
benlk commented Dec 3, 2015

Reopening this for reasons discussed in #largo-dev Slack:

This email-button fix affects floating social buttons, which aren't in master yet, and merging the email fix but not floating social buttons into master will mean that the same thing gets done by different commits in each. It's probably simpler and safer and saner to merge this PR in develop

Also, while this fix makes it possible to receive the email popup from clicking child elements of .custom-social-button, it is possible to get the email popup in the current master by clicking carefully, so it's not super broken, and not enough people have complained.

@benlk benlk reopened this Dec 3, 2015
@benlk benlk modified the milestones: 0.5.4, hotfix Dec 3, 2015
benlk added a commit that referenced this pull request Dec 10, 2015
@benlk
Merge pull request #998 from INN/961-broken-email-button
ca5131f 
Broken email button fixes
@benlk benlk merged commit ca5131f into develop Dec 10, 2015
@benlk benlk mentioned this pull request Dec 10, 2015
Closed
@benlk benlk deleted the 961-broken-email-button branch February 3, 2016 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@rnagle rnagle

Labels
priority: high Either blocks work on a priority-normal task or a solution here informs other work.
Projects
None yet
Milestone
0.5.4
Development

Successfully merging this pull request may close these issues.
3 participants
@benlk @rnagle @aschweigert
0