webViewEventHandlers: Add ability to copy links to Clipboard. by ishammahajan · Pull Request #3404 · zulip/zulip-mobile

ishammahajan · 2019-03-15T01:43:54Z

There is no way to copy only links from a message at the moment, which
makes it hard for some users to copy only the links to share with other
people. There could be two solutions to this. One is that another menu
option could be provided to copy only the link in your message, which
is not a correct approach as immediately some problems come to mind.
For example, if there were two links in the message, which one would
the app copy? Another solution to this could be what commercial
applications already apply, which is if you hold the link text, it
gets copied. Which is what has been implemented here. I have added
another event to MessageListEvent which abstracts a long press on a
url, and edited the long press listener on js.js a little bit.

I have only conducted the automated tests here (besides manually
checking if copying links works by building the application).

I have edited the js.js file and tweaked the long press functionality
to make it possible to copy links on long press. Long press itself is
very shaky to capture in a webview on mobile, which is why I was
compelled to tweak it in this manner. Please consider this before
merging.

Fixes #3385.

borisyankov · 2019-03-17T21:27:26Z

@@ -1,4 +1,4 @@
-/* @flow strict */


Did you intend this change?
This is relaxing our type-checking and is undesirable.
Rework the code so this is not needed.

This and the below edits were done in order to abstract the Clipboard method for setting the string. import { Clipboard } from 'react-native' caused issues with the type-checking, and I could not find a more appropriate file to insert the abstraction into. I don't know what to do otherwise, creating another file just for Clipboard seems unwise. Please suggest.

borisyankov · 2019-03-17T21:29:06Z

@@ -1,4 +1,7 @@
-/* @flow strict */


Same as previous comment, please do not lower the strictness of the code.
If you are trying to solve a type issue, there is likely an alternative solution.
You can ask for help if you can't figure it out.

borisyankov · 2019-03-17T21:29:46Z

 export const transformToEncodedURI = (string: string): string =>
  string.replace(/\.\d/g, (match, p1, p2, offset, str) => `%${match[1]}`);
+
+export const copyStringToClipboard = (string: string): void => Clipboard.setString(string);


I am skeptical that abstracting away the use of Clipboard is useful in this case.

I was viewing a PR to gain some intuition as to what I might have left out, another maintainer had suggested as much abstraction as possible. And the reason they gave was that there are multiple instances where Clipboard might be used, and if it's abstracted it would be easier to refactor in case the parent API changes. I do agree though, Clipboard may not need to be abstracted. It has only two instances of use. Not worth losing the type-checking over. I'll rollback to the previous commit.

borisyankov · 2019-03-17T21:55:52Z

  messageId: number,
 |};

+type MessageListEventLongPressUrl = {|


You might want to consider adding the href parameter to MessageListEventLongPress and add another target as url/link.

That seems to be a good idea. Thanks! 🙂

borisyankov · 2019-03-17T21:59:22Z

  if (!lastTouchEventTimestamp || Date.now() - lastTouchEventTimestamp < 500) {
-    return;
-  }
+    if (target.matches('a')) {


You already gave some explanation that there is weirdness in how long-press works with links.
I am not sure if this solution is the right one (I really don't know :) )
It will be better to extract this behavior in a separate commit.

The behavior of long press on links is likely to differ between Android and iOS.
Make sure to test on both platforms and to mention that in the commit message.

Long-press works weirdly with mobile devices, since MouseEvents sort of conflict with TouchEvents. This solution seems to be the only one available since there are various issues introduced if the longPress events for URLs and the messages/headers are decoupled (the action sheet launches and the URL is opened, instead of one of them happening in each case). I could possibly use a library which exposes longPress properly in js (as mentioned in the comments preceding this code), but all of the libraries I found either didn't work properly (not as desired for this use-case) or was poorly maintained.

Plus, with this change (coupled with the previous comment about changing MessageListEventLongPress to include another target for link) the code becomes much less cluttered, since all the three targets are confined to one scope.

I will check out long presses on Android and update the commit message (tested on iOS for now). Thanks!

This bit of logic doesn't belong here inside this conditional. The reason is that it makes the code much more complex to understand.

One way to see that: try to describe what the meaning of this code is (this if (target.matches('a')) ... block, when it's located here), and to explain why it works correctly. I think you'll find that is pretty complicated to do successfully -- and moreover that it contradicts what's there in the text, in particular the name handleLongPress.

I agree with Boris's advice to separate out this change in its own commit. I don't yet understand what your purpose is in moving this bit of code here from the click handler, so it requires some explanation... and it will be a lot easier to successfully explain it if it's in its own commit.

(Like I said above, it still won't belong quite here, but once we understand what this change is intended to do we'll be better able to advise on how to adjust it.)

I see the point that it's hard to understand, but the logic gets pretty messed up if we try and un-bind it.

An issue arises where both short press and long press can be triggered from clicking the message.

To remedy the understandability of the code, I have made a few changes within the code.

Changed the method name from handleLongPress to handleClicksOnMessage since that is basically what the method is doing, it handles behaviour of a users interaction with the body of the message.

Added a comment detailing what the problem is, and how the implemented logic works.

// This method is a little convoluted, since it aims to provide a solution
// to the long press event in-availability. If this solution is not
// provided, both the short presses and the long presses can trigger at the
// same time when a user tries to perform an action on a message.
//
// The first if checks if the person is clicking the link, and if
// the holding time is less than 500ms.
// If both of the conditions check, and the target of the click is a URL,
// this script returns a sendMessage with type url, else, script
// returns with sendMessage type longPress, and an href if the element
// the longPress is performed on is a link.

That said I did move the change into its own commit.

Said issue can be experienced like this.

gnprice

Thanks @ishammahajan for this contribution, and @borisyankov for the review!

I just read through the current version; comments below.

BTW @ishammahajan I especially appreciate the detailed commit messages in this and your other PRs. 😃

gnprice · 2019-03-20T02:36:40Z

+  if (target === 'link') {
+    Clipboard.setString(href);
+    showToast('Link copied to Clipboard');
+  } else {


This can be made a bit clearer with an early return:

if (...) { ... showToast(...); return; } const message = ...

Among other things, that keeps the "main story" aligned on a single column, with just the "side notes" indented.

I had actually tried that, but eslint goes crazy with eslint(no-useless-return) and eslint(no-else-return).

Hmm -- can you share the exact code that gives you that? (Perhaps in chat, for more-interactive debugging.) We use the early-return pattern often, and don't get those lint errors.

I sometimes have seen lint errors like those on code that was better than the alternative, though; it's possible we should just turn those rules off.

In general, if a linter complains about some code you think is good and doing what the linter says makes it worse, feel free to just disable it locally (preferably like eslint-disable-line specific/rule -- git grep eslint-disable for examples) and submit the code that way. The disabling will likely be something to discuss, so do mention it in the commit message or the PR.

bump - still open

gnprice · 2019-03-20T02:37:02Z

+const handleLongPress = (
+  props: Props,
+  _: GetText,
+  target: string,


This can be more specific -- 'message' | 'header' | 'link', just like above.

gnprice · 2019-03-20T02:37:54Z

+      type: 'longPress',
+      target: target.matches('.header') ? 'header' : target.matches('a') ? 'link' : 'message',
+      messageId: getMessageIdFromNode(target),
+      href: target.matches('a') ? target.getAttribute('href') : '',


Let's not use '' as a symbol for "no string here" -- it should only be used when we really want to say there's a string which is empty.

Instead, null is good. (Or undefined, but that doesn't translate through JSON.)

gnprice · 2019-03-20T02:39:59Z

+) => {
+  if (target === 'link') {
+    Clipboard.setString(href);
+    showToast('Link copied to Clipboard');


This should be translated -- use _.

gnprice · 2019-03-20T02:46:34Z

+  messageId: number,
+  href: string,
+) => {
+  if (target === 'link') {


There's some redundancy here that creates a possibility of type errors, and with some adjustment of definitions we can make them impossible instead.

Specifically, once you see target === 'link' you're relying on the assumption that that means href will be valid. Which happens to be true in this version of the code, but it's not expressed in the types so the type-checker can't ensure it stays true, even though it's basically a type fact.

One way to fix this would be to just use target: 'message', and let a non-null value of href mean it's actually a link. This would work OK.

Another would be to make another event type, like copyLink. This can have href required... and also it doesn't need messageId, which we aren't using here. This is a bit more work, but also more explicit.

In the original version of this PR, I had implemented solution two, but @borisyankov suggested that

You might want to consider adding the href parameter to MessageListEventLongPress and add another target as url/link.

I agree with him; I don't think we require another event whose only difference with longPress is that it's a link, it'll increase code complexity unnecessarily.

Solution one suggests to use target: 'message' and check if href is not null, but I don't think this is the optimal approach, since it will make the code harder to understand (too abstract).
I have implemented the modified solution one (checking if href is not null) but I feel like I haven't understood the entire problem here. Could you please explain a bit more, or perhaps provide a link to an article which describes this in a more generic way?

I have implemented the modified solution one (checking if href is not null)

Hmm, I don't see an update -- did you forget to push? 😉 Or maybe I don't understand what you mean by this line.

Could you please explain a bit more, or perhaps provide a link to an article which describes this in a more generic way?

One common slogan for this principle is "make illegal states unrepresentable".

Here's one blog post I found (from a search just now) that explains it with another example:
https://oleb.net/blog/2018/03/making-illegal-states-unrepresentable/

Hmm, I don't see an update -- did you forget to push? 😉

Sorry about that, I meant I had those changes in the working directory.
Pushed now though!

Also thanks so much for the explanation! 😄

gnprice

Thanks @ishammahajan for this update!

Some quick comments below. Next I'll read up on the long-press vs. short-press issue, and your nice new separated-out commit for that.

gnprice · 2019-03-20T21:25:21Z

+  if (target === 'link') {
+    Clipboard.setString(href);
+    showToast('Link copied to Clipboard');
+  } else {


bump - still open

gnprice · 2019-03-20T21:27:24Z

+  messageId: number,
+  href: string,
+) => {
+  if (href !== null) {


Cool, but this revision is incomplete -- there's still the redundant distinction of link from message, appearing in several places in the code.

Oh, also the commit message still describes your v1 design:

I have added another event to `MessageListEvent` which abstracts a long press on a url, and edited the long press listener on `js.js` a little bit.

I purposely left the distinction between link and message for the sake of readability of code. Do you think I should remove it?

Fixed the commit message (in WIP)! :)

gnprice · 2019-03-20T21:47:08Z

On the long-press logic: thanks, this is helpful for understanding your thinking! Both the new separate commit, and especially the video (https://i.imgur.com/phH6B5C.mp4 ).

The first question I'd ask on that bug is: why is it specific to the new behavior on links? Why doesn't it also apply to e.g. long-pressing a topic bar (.header)?

I don't see a reason why it should be specific to links, but your fix only applies to links.

Also I can't seem to actually reproduce the issue; not on Android with your first commit (and not your second), and neither on Android nor iOS with the latest release, if I try it on things like topic bars. I notice your video is on iOS -- would you check if it reproduces for you on Android? If it's platform-specific, that'll be a clue.

gnprice

Some style specifics on the new commit. (The main discussion is in my previous comment, though.)

gnprice · 2019-03-20T21:49:32Z

 });

-const handleLongPress = (target: Element) => {
+const handleClicksOnMessage = (target: Element) => {


This name is inaccurate:

This isn't specific to messages -- notice the target: 'header' case.

The click handler, above, is mostly about messages, so "on message" doesn't describe the distinction between them at all.

I agree. At this point it seems better just to let it be handleLongPress.

gnprice · 2019-03-20T21:52:34Z

+   * If both of the conditions check, and the target of the click is a URL,
+   * this script returns a `sendMessage` with type `url`, else, script
+   * returns with `sendMessage` type `longPress`, and an href if the element
+   * the `longPress` is performed on is a link.


This paragraph is just reciting the same information as the code. This kind of comment is almost always of negative value -- it doesn't really make the code any easier to understand (so there's little positive value to begin with); it adds more length and more things to read (which is a cost); and most dangerously, it is very likely to become untrue in the future when the code changes, and actively mislead someone.

(The paragraph above it is much more helpful, though! That's information that isn't easy to get from just reading the code, and is higher-level so less sensitive to becoming wrong when the code changes.)

I agree, I'll remove it.

ishammahajan · 2019-03-21T23:00:30Z

Your reproduction of the issue seems to be correct. With the first commit it works on only Android, and with the second it works on both (because the second commit was made to fix that issue).

The issue arises due to an unknown reason I have not been able to discern for the better part of half a day due to the following reasons:

I initially thought it had something to do with the touch being held even after the longPress is complete. On Android if we hold a link (with only the first commit) the highlight which occurs on holding the link goes away as soon as the longPress completes. While on iOS, it seems to persist.
But that was ruled out by the fact that the same thing was not happening to headers.
Then I thought that the ActionSheet which pops up on clicking the message was blocking out the rest of the touch on the message/link, which is why on the header (since the ActionSheet got triggered) the doNarrow wasn't occurring.
That was proven to be wrong because if we disabled the ActionSheet from popping up, and then we longPress the header, the short press on the header did not trigger (it should according to this hypothesis, since the ActionSheet has been disabled and there is nothing to block the touches now)!

In all, this is very confusing behaviour which cannot be replicated for Android and only exists for iOS. It can be easily fixed with commit no. 2 for iOS as well (it could perhaps be explained in high-level why we have done so in a comment).

Update: There is a bug with the second commit because of which, if an .avatar is clicked, the long press action sheet for the message it belonged to also gets clicked. I may have to rethink the logic for copying links, because I cannot discern why this might happen. Again, this issue only occurs on iOS, not on Android.

See discussion for context (zulip#3404 (comment)).

gnprice · 2019-03-23T00:29:36Z

Cool, thanks for those notes from further debugging!

Definitely interesting that the issue is iOS-only. And also interesting that it applies to links but not to topic headers.

This seems like an especially interesting clue:

On Android if we hold a link (with only the first commit) the highlight which occurs on holding the link goes away as soon as the longPress completes. While on iOS, it seems to persist.

... Oho, I can actually reproduce the issue on iOS on the current release! So this is a live bug today.

Find a message with an image in it. Scroll so it's near the top of the message list.
Press and hold, long enough for the action sheet to show up.
- Like you said, the image stays highlighted.
- You want it to still be in view, not hidden by the action sheet -- this is why we scrolled earlier to put it near the top.
Now let go.
We navigate to the lightbox -- just as if you'd done a normal, non-long press.

It seems to be important that after the action sheet shows up you wait for a moment before letting go, like a few hundred ms. If you let go as soon as the long-press triggers -- like I'd usually do -- the bug doesn't show up.

The bug also reproduces on

the link just above an image
internal links to streams/etc. -- if they're in the old format, i.e. not affected by Internal links to streams and narrows don't work #2760, so they actually work in the first place

This would be good to do some further debugging/research and get to the bottom of.

@borisyankov , do you have any suggestions?

I wonder also if the new WebView implementation (coming soon, #3296) might affect this issue, one way or another.

See discussion for context (zulip#3404 (comment)).

The long press logic is very subtle, as described in `js.js`, and the logic around `lastTouchEventTimestamp` is even more obscure. There was a bug in iOS, where the a shortPress, or a `click`, was triggered after releasing the finger after a `longPress` event. This commit fixes this bug by introducing a flag which detects whether a long press was triggered before the detection of this click. While this could be potentially implemented without an extra flag and with the already existing `lastTouchEventTimestamp`, on trying to implement it, there arise a slew of other bugs. Besides, if a new flag was not used, the logic around `lastTouchEventTimestamp` would become unreadable. See zulip#3404 (comment) for more details.

ishammahajan · 2019-03-24T15:05:10Z

@gnprice thanks for the review!

I have tried to debug this further, but to no avail. I still have no idea why this is the case only in iOS. I did find something peculiar though, in UIWebKit, there is an issue with clicks and the delay with respect to them triggering. Is there a possibility that such an issue might be interfering with our longPress logic?

... Oho, I can actually reproduce the issue on iOS on the current release! So this is a live bug today.

I have managed to fix this bug with the latest commits, and have separated them for the sake of clarity. The change was basically to add a flag which detects if a long press was triggered before the detected click, as in the multiple iterations I tried this seemed to be the cleanest and easiest to understand.

I wonder also if the new WebView implementation (coming soon, #3296) might affect this issue, one way or another.

At first glance on the change-log (of the ported library), it doesn't seem like this will be why this issue happens, but I will regardless test whether it fixes it without the flag used above.

Here's the recording which shows the fixed behaviour.

gnprice · 2019-03-26T21:28:08Z

Thanks @ishammahajan ! This solution to the extra-click issue is much cleaner 😄 , and I like the small diffstat:

 src/webview/js/js.js           |   9 +++++++++

This version breaks things on Android, though. (And I suspect also some of the time on iOS -- basically anytime the existing bug didn't trigger.) To solve it, I recommend you:

Think carefully about the state machine of hasLongPressed. In particular, when does it get set to false?
Test your code out on Android as well as on iOS.
Fix the bug. 🙂
Repeat until you (a) can't reproduce any bug on either platform, and (b) can convince yourself that the state machine makes sense and should mean there isn't a bug.

Once that's in shape, I think we'll be basically ready to merge this PR.

`ad77730` webViewEventHandlers: Add ability to copy links to Clipboard.

Small comments on this commit:

Let's actually put this commit after the bugfix, not before. That's a bit more logical because that way we don't have an intermediate step where the bug is still there and has been made more visible and easy to trigger.

+  href: string,

This needs to be updated to reflect the type of what we actually send for these events.

(Meanwhile, the fact that this oversight is even possible demonstrates a gap in our type-checking! It'd be good to fix that. Looks like we've disabled the type-checker at sendMessage, by writing const sendMessage = (msg: Object) => etc., with Object which is basically never the right answer in the end... hmm, I may just go fix that now.)

gnprice · 2019-03-26T21:58:48Z

(Meanwhile, the fact that this oversight is even possible demonstrates a gap in our type-checking! It'd be good to fix that. Looks like we've disabled the type-checker at sendMessage, by writing const sendMessage = (msg: Object) => etc., with Object which is basically never the right answer in the end... hmm, I may just go fix that now.)

And done, in dd03939 . When you rebase, you'll find that Flow gives you an appropriate error for this. 🎉

The long press logic is very subtle, as described in `js.js`, and the logic around `lastTouchEventTimestamp` is even more obscure. There was a bug in iOS, where the a shortPress, or a `click`, was triggered after releasing the finger after a `longPress` event. This commit fixes this bug by introducing a flag which detects whether a long press was triggered before the detection of this click. While this could be potentially implemented without an extra flag and with the already existing `lastTouchEventTimestamp`, on trying to implement it, there arise a slew of other bugs. Besides, if a new flag was not used, the logic around `lastTouchEventTimestamp` would become unreadable. See zulip#3404 (comment) for more details.

ishammahajan · 2019-03-27T20:56:34Z

@gnprice thanks for being so patient! 🙂

Sorry about the silly bug which occurred. I should have tested the behaviour for all possible cases before pushing.

The bugs have been fixed. I have tested the commits with both Android (emulator and device) and iOS (and will do so moving forward.)
Have rearranged the commits to put the fix first.
href: string,
This needs to be updated to reflect the type of what we actually send for these events.
- This is now href: string | null.
I think you'll find the bug fixes sweet with the updated commit as well (+10 insertions) 😄 .

The long press logic is very subtle, as described in `js.js`, and the logic around `lastTouchEventTimestamp` is even more obscure. There was a bug in iOS, where the a shortPress, or a `click`, was triggered after releasing the finger after a `longPress` event. This commit fixes this bug by introducing a flag which detects whether a long press was triggered before the detection of this click. While this could be potentially implemented without an extra flag and with the already existing `lastTouchEventTimestamp`, on trying to implement it, there arise a slew of other bugs. Besides, if a new flag was not used, the logic around `lastTouchEventTimestamp` would become unreadable. Fixes zulip#3429.

There is no way to copy only links from a message at the moment, which makes it hard for some users to copy only the links to share with other people. There could be two solutions to this. One is that another menu option could be provided to copy only the link in your message, which is not a correct approach as immediately some problems come to mind. For example, if there were two links in the message, which one would the app copy? Another solution to this could be what commercial applications already apply, which is if you hold the link text, it gets copied. Which is what has been implemented here. I have modified the already existing `MessageListEventLongPress` to include functionality for long-pressing on links. Tests have been conducted on iOS simulator and Android emulator. Functionality has also been checked for Android on a real device. This commit also made conspicuous a related bug which already affected long-presses on iOS (zulip#3429), fixed separately in the previous commit for the sake of clarity. Fixes zulip#3385.

Happily, for many languages we already have a translation for this string.

gnprice · 2019-03-28T01:47:20Z

And merged. Thanks again @ishammahajan for all your work on this!

Small style nit on these excellently detailed commit messages: please wrap prose in paragraphs at about 68 columns (no more than 70 columns max). You may find it helpful to (a) configure Git to use your preferred editor, with e.g. the EDITOR environment variable or git config --global core.editor; (b) configure the editor to automatically wrap text at the appropriate width (e.g. (push 'turn-on-auto-fill text-mode-hook) in ~/.emacs.d/init.el if you use Emacs; all text editors have some way to do this.)

ishammahajan · 2019-03-28T07:29:09Z

Thanks for all the help as well! I'll try to remember these pieces of advice with new PRs. The textwidth setting accomplishes hard wrap for me on vim, I didn't initially know text editors have hard wrap available.

@gnprice

@gnprice has commented that commit message lines should wrap at about 68 columns, but not exceed 70. zulip/zulip-mobile#3404 (comment)

@gnprice

@gnprice has commented that commit message lines should wrap at about 68 columns, but not exceed 70: zulip/zulip-mobile#3404 (comment) A discussion followed at https://chat.zulip.org/#narrow/stream/3-backend/topic/commit.20line.20length and that was agreed on for the commit message body, but noted that a length of 76 was acceptable for the summary because of the single-line constraint and the fact that it ends in a period, making it clear where it ends.

zulip/zulip-mobile#3404 (comment) A discussion followed at https://chat.zulip.org/#narrow/stream/3-backend/topic/commit.20line.20length and that was agreed on for the commit message body, but noted that a length of 76 was acceptable for the summary because of the single-line constraint and the fact that it ends in a period, making it clear where it ends.

borisyankov reviewed Mar 17, 2019

View reviewed changes

Comment thread src/webview/js/generatedEs3.js Outdated

borisyankov reviewed Mar 17, 2019

View reviewed changes

ishammahajan force-pushed the copy-link-only branch 2 times, most recently from 7980036 to ccfeca9 Compare March 18, 2019 18:23

gnprice reviewed Mar 20, 2019

View reviewed changes

ishammahajan force-pushed the copy-link-only branch from ccfeca9 to e24f290 Compare March 20, 2019 20:19

gnprice reviewed Mar 20, 2019

View reviewed changes

ishammahajan force-pushed the copy-link-only branch from e24f290 to 573822e Compare March 20, 2019 22:42

This was referenced Mar 20, 2019

usability: Add the possibility to copy only a link from a message. #3405

Closed

Support copying just a link URL, rather than whole message text #2249

Closed

ishammahajan force-pushed the copy-link-only branch from 573822e to 16ed373 Compare March 21, 2019 23:21

ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this pull request Mar 21, 2019

fix issue where both link is both copied to clipboard and opened

16ed373

See discussion for context (zulip#3404 (comment)).

gnprice mentioned this pull request Mar 23, 2019

messageActionSheet: Add option to copy link of message to clipboard #3412

Closed

ishammahajan force-pushed the copy-link-only branch from 16ed373 to fd18914 Compare March 24, 2019 14:29

ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this pull request Mar 24, 2019

fix issue where both link is both copied to clipboard and opened

8be8252

See discussion for context (zulip#3404 (comment)).

ishammahajan force-pushed the copy-link-only branch from fd18914 to 69db4e0 Compare March 24, 2019 14:36

ishammahajan force-pushed the copy-link-only branch from 69db4e0 to 9ae2252 Compare March 27, 2019 20:26

gnprice mentioned this pull request Mar 28, 2019

Extra click on long-press, in message list on iOS #3429

Closed

ishammahajan force-pushed the copy-link-only branch from 9ae2252 to 062f334 Compare March 28, 2019 01:41

ishammahajan and others added 3 commits March 27, 2019 18:41

i18n: Sync translations for new string from Transifex.

96f285f

Happily, for many languages we already have a translation for this string.

gnprice force-pushed the copy-link-only branch from 062f334 to 96f285f Compare March 28, 2019 01:43

gnprice merged commit 96f285f into zulip:master Mar 28, 2019

chrisbobbe mentioned this pull request Aug 31, 2019

docs/version-control: Set commit message line limit to 70. zulip/zulip#13119

Closed

Uh oh!

Uh oh!

Conversation

ishammahajan commented Mar 15, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishammahajan Mar 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishammahajan Mar 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishammahajan Mar 20, 2019 •

edited

Loading

ishammahajan Mar 20, 2019 •

edited

Loading

ishammahajan commented Mar 21, 2019 •

edited

Loading

`ad77730` webViewEventHandlers: Add ability to copy links to Clipboard.