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

Mark gplus share button as obsolete and don't render it. #23682

Merged
merged 3 commits into from
Aug 5, 2019
Merged

Mark gplus share button as obsolete and don't render it. #23682

merged 3 commits into from
Aug 5, 2019

Conversation

cramforce
Copy link
Member

  • Introduces the concept of an obsolete share button
  • Renders those as display:none
  • Marks gplus (RIP) as obsolete.

Fixes #21743

@cramforce
Mark gplus share button as obsolete and don't render it.
62762a6 
- Introduces the concept of an obsolete share button
- Renders those as `display:none`
- Marks gplus (RIP) as obsolete.

Fixes ampproject#21743
@cramforce cramforce requested a review from sparhami August 5, 2019 16:54
@cramforce cramforce merged commit 4140813 into ampproject:master Aug 5, 2019
@cramforce cramforce deleted the g-plus-RIP branch August 5, 2019 23:40
@newmuis
Copy link
Contributor

newmuis commented Aug 7, 2019 •
edited
Loading

@cramforce out of curiosity, did you test this with stories? I suspect this will break the UI.

@cramforce
Copy link
Member Author

Nope

@newmuis
Copy link
Contributor

newmuis commented Aug 7, 2019 •
edited
Loading

Testing it out real quick by just display: noneing the <amp-social-share> element, it looks like this leaves a gap in the share sheet where there icon was supposed to be. We structure it as a list, with the list items having the size; we can refactor to not do that.

But, it seems pretty likely that this will make the cut and the fix won't, so we'll need to decide whether this is important enough for a cherrypick (into canary)

@cramforce
Copy link
Member Author

We can certainly revert it and then fix forward

@cramforce
Copy link
Member Author

Where can I test this? It seems most of the example stories don't render and those that do only show the URL share link.

thekorn pushed a commit to edelight/amphtml that referenced this pull request Sep 11, 2019
@cramforce
Mark gplus share button as obsolete and don't render it. (ampproject#…
677a8bd 
…23682)

* Mark gplus share button as obsolete and don't render it.

- Introduces the concept of an obsolete share button
- Renders those as `display:none`
- Marks gplus (RIP) as obsolete.

Fixes ampproject#21743

* Fix lint

* Fix lint
@mszylkowski mszylkowski mentioned this pull request Nov 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sparhami sparhami sparhami approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove G+ from documentation of amp-social-share
4 participants
@cramforce @newmuis @sparhami @googlebot