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.

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

Whatsapp button should check if mobile #194

Merged
merged 2 commits into from
Dec 19, 2018
Merged

Conversation

madkoding
Copy link
Contributor

Checking if mobile, url would be 'api'. Instead in desktop browser would open 'web' whatsapp to share

Checking if mobile, url would be 'api'. Instead in desktop browser would open 'web' whatsapp to share
Copy link
Collaborator

@aautio aautio left a comment

Choose a reason for hiding this comment

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

Thank you @madkoding for the pull request. It seems to be working. This would help us out on multiple WhatsApp-related issues.

There are a few things where the code should be modified before merging. Would you be willing to do these changes?

  1. Please execute npm run lint to see which lines do not follow the required formatting rules.

  2. Please shorten and simplify the mobile/tablet detection function. The current regex on line 11 matches too many devices. According to https://faq.whatsapp.com/en/bb/26000006/ there are only 5 operating systems that would need to be checked.

@aautio
Copy link
Collaborator

aautio commented Dec 12, 2018

I pushed simplification to mobile/tablet check. It covers the platforms where Whatsapp is available.

The code now passes eslint.

@aautio aautio added the v3.0.0 Changes to be landed in version 3.0.0 label Dec 12, 2018
@aautio aautio changed the base branch from master to v3 December 19, 2018 19:40
@aautio aautio merged commit d328f9e into nygardk:v3 Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3.0.0 Changes to be landed in version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants