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

Don't show hyperlink for non-number text #5

Open
gitcommitshow opened this issue Jun 26, 2021 · 9 comments
Open

Don't show hyperlink for non-number text #5

gitcommitshow opened this issue Jun 26, 2021 · 9 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@gitcommitshow
Copy link
Owner

No description provided.

@gitcommitshow gitcommitshow added the good first issue Good for newcomers label Jun 26, 2021
@nishit-prasad
Copy link

Hello! I am new to open source and would like to work on this. Can I take this up?

I can contribute more to issues/features/docs etc. once I understand and get this set up.

@gitcommitshow
Copy link
Owner Author

gitcommitshow commented Jun 27, 2021

Can I take this up?

Absolutely, let me know if you need any support from my side

@nishit-prasad
Copy link

nishit-prasad commented Jun 27, 2021

@gitcommitshow
Thank you!

Question, for this issue - is it a better idea that the right-click menu shows up only when you select a string that is a number? (Not able to see hyperlink anywhere - if I am understanding this right)

@gitcommitshow
Copy link
Owner Author

Question, for this issue - is it a better idea that the right-click menu shows up only when you select a string that is a number? (Not able to see hyperlink anywhere - if I am understanding this right)

No. Right click context menu should always be shown. Because this is the default behaviour on browser that people expect.
Right now, we add the selected text as one of the item in this right click context menu.

What we want to do now is to add that menu item only if the selected text contained at least 10 digits.

@nishit-prasad
Copy link

Got it.

So currently in non-number string such as foo, I see the menu item as Whatsapp: foo.

So would you like to change it to just Whatsapp?

@gitcommitshow
Copy link
Owner Author

So currently in non-number string such as foo, I see the menu item as Whatsapp: foo.
So would you like to change it to just Whatsapp?

We can do one of the following

A. Change the text to Whatsapp: <Invalid No.>
B. Don't show Whatsapp.... at all

I would vote for B option, but do whatever you think is the best for good user experience.

@gitcommitshow
Copy link
Owner Author

Also check the issue #9 , we can fix that issue right after fixing this current issue. So having both of these fixes #5 and #9 will improve the user experience significantly IMO

@nishit-prasad
Copy link

Thank you for the clarification.

So based on what I understand, if we go with option B then the menu item won't be displayed if the number is not a valid 10-digit number. Right?

And for #9 if we go with tooltip, we can just render WhatsApp icon - this way it can be less annoying instead of rendering text "Whatsapp" on tooltip.

@gitcommitshow
Copy link
Owner Author

So based on what I understand, if we go with option B then the menu item won't be displayed if the number is not a valid 10-digit number. Right?

Yes. And we need to keep in mind - there are different formats for writing numbers that are correct e.g. 99999999, 9999-999-999, +01-9999999999, +019999999999, +920-9999999999 and many more.

And for #9 if we go with tooltip, we can just render WhatsApp icon - this way it can be less annoying instead of rendering text "Whatsapp" on tooltip.

Agree. We can put whatsapp icon next to the number so user can see the no. as well as the clickable icon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants