Skip to content

Conversation

@aautio
Copy link
Collaborator

@aautio aautio commented Feb 3, 2019

Here is a pull request from v3 branch containing all pending changes. Updated CHANGELOG.md and README.md are included.

  • A default aria-label is generated for the share buttons. It is based on the network name. Use additionalProps to override.
  • Removed support for Google+.
  • Removed support for <LinkedInShareCount/>. LinkedIn no longer provides an API to fetch share counts.
  • Removed title and description from <LinkedInShareButtons/>.
  • Fix: <EmailShareButton/> includes body before url.
  • Fix: <EmailShareButton/> and <ViberShareButton/> allow customization of separator.
  • Thanks @finppp, @madkoding and @andrewl913!

Sorry it took so long to prepare this - but better late than never I guess. @nygardk would you review this, merge and publish to NPM if all looks good? A correct date should be set to CHANGELOG.md line 3: ## 3.0.0 (Feb ?, 2018) while merging.

I should have time to do fixes/changes within the upcoming week if any are required.

andrewl913 and others added 22 commits November 28, 2018 12:23
Checking if mobile, url would be 'api'. Instead in desktop browser would open 'web' whatsapp to share
Update EmailShareButton.js
Whatsapp button should check if mobile
Deprecations of LinkedIn and Google+, default aria-labels
Viber shares no longer prepend space and expose the separator as a prop.
Fully removing Google+ as all their APIs are shut down within 90 days
@aautio aautio requested a review from nygardk February 3, 2019 17:16
Copy link
Owner

@nygardk nygardk left a comment

Choose a reason for hiding this comment

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

I think this looks great!

I'd like to get a couple of additional minor things to this release:

  • Remove assertion from WeiboShareButton for the image (image shouldn't be required?).
  • Add centerWindowOnScreen prop (or similar) to share buttons to control whether the opened window should be centered on the screen's center or to parent windows center position (current default). E.g. Weibo's share window seems to have logic of it's own that makes it jump to the center of the screen instead of the parent center shortly after it's opened, which is ugly behaviour. I'd like to make the screen center default for such share buttons.

I'll work on these ^ PRs as soon as possible!


Additionally, looking forward for version 4 we could look at adding Prettier and TypeScript. What do you think? TypeScript would make the library easier to use with the always-up-to-date built-in types (as opposed to types from DefinitelyTyped).

@aautio
Copy link
Collaborator Author

aautio commented Feb 5, 2019

Everything sounds great!

centerWindowOnScreen might require testing with single and dual monitor -setups to get it working in a consistent manner?


I like the Javascript -> Implicit Project Config: Check JS -feature of Visual Studio Code to run TypeScript checks for plain .js-files. It has been a huge help with JS projects.

The implicit check works just great for most NPM packages - but not yet with react-share when I'm importing things this way:

import WhatsappShareButton from 'react-share/lib/WhatsappShareButton'

I could pitch in to the TypeScript-conversion with ☝️in my mind.

@aautio
Copy link
Collaborator Author

aautio commented Mar 22, 2019

@nygardk I've updated the changelog and merged everything to the branch v3. The actual release date needs to be added there - otherwise everything should be listed.

Would you mind building and publishing a 3.0.0-beta.0 from v3 to npm?

@nygardk
Copy link
Owner

nygardk commented Mar 24, 2019

@aautio This is now published as 3.0.0-beta.0 in NPM. Installable with npm i react-share@beta.

@aautio
Copy link
Collaborator Author

aautio commented Mar 24, 2019

@aautio This is now published as 3.0.0-beta.0 in NPM. Installable with npm i react-share@beta.

Nice! I'll test it next week within a few sites and get back to you.

@nygardk
Copy link
Owner

nygardk commented Apr 9, 2019

@aautio This is now published as 3.0.0-beta.0 in NPM. Installable with npm i react-share@beta.

Nice! I'll test it next week within a few sites and get back to you.

Any update on testing 🙂?

@aautio
Copy link
Collaborator Author

aautio commented Apr 18, 2019

Hi @nygardk, sorry for dropping the ball. I've left the country for a few weeks without my laptop. I'll be back around 2nd or 3rd of May. Would you be ok with waiting that far?

@nygardk
Copy link
Owner

nygardk commented Apr 18, 2019

@aautio
Hey, of course, it's ok :). Have fun abroad!

@aautio
Copy link
Collaborator Author

aautio commented May 13, 2019

@nygardk - sorry the delay. But I've finally had the chance to test the v3 beta.

Short summary: It just works 👍

Should we go forward and remove the beta label, merge this pr and publish a 3.0.0 version to NPM?

@nygardk nygardk merged commit 697fb96 into master May 15, 2019
@nygardk nygardk deleted the v3 branch May 15, 2019 06:56
@nygardk
Copy link
Owner

nygardk commented May 15, 2019

Published!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants