UX: Remove share as link fallback on touch devices (PR #9332)

[pmusaraj]

Previously, there were three workflows when sharing a topic or a post:

• sharing URL displayed in popup or modal, in an input or textarea element
• native sharing dialog on touch devices with native sharing support
• and a fallback to the URL displayed as link element on touch devices with no native sharing support

This PR removes the third option. We were only displaying this on non-iOS/non-Android touch devices like laptops with a touch screen, and it was buggy, see https://meta.discourse.org/t/user-experience-on-discourse-with-optional-touch-input/40190/24 for details.

GitHub

[jjaffeux]

app/assets/javascripts/discourse/components/share-panel.js

@@ -43,31 +42,12 @@ export default Component.extend({
 
   didInsertElement() {
     this._super(...arguments);
-
-    const shareUrl = this.shareUrl;
-    const $linkInput = $(this.element.querySelector(".topic-share-url"));
-    const $linkForTouch = $(
-      this.element.querySelector(".topic-share-url-for-touch a")
-    );
-
-    schedule("afterRender", () => {
-      if (!this.capabilities.touch) {
-        $linkForTouch.parent().remove();
-
-        $linkInput
-          .val(shareUrl)
-          .select()
-          .focus();
-      } else {
-        $linkInput.remove();
-
-        $linkForTouch.attr("href", shareUrl).text(shareUrl);
-
-        const range = window.document.createRange();
-        range.selectNode($linkForTouch[0]);
-        window.getSelection().addRange(range);
-      }
-    });
+    window.setTimeout(() => {

setInterval/setTimeout should never be used, we still have some in the codebase I think… but prefer Ember.run.later

[jjaffeux]

app/assets/javascripts/discourse/components/share-popup.js

@@ -35,21 +35,14 @@ export default Component.extend({
   },
 
   _focusUrl() {
-    const link = this.link;
-    if (!this.capabilities.touch) {
-      const $linkInput = $("#share-link input");
-      $linkInput.val(link);
-
-      // Wait for the fade-in transition to finish before selecting the link:
-      window.setTimeout(() => $linkInput.select().focus(), 160);
-    } else {
-      const $linkForTouch = $("#share-link .share-for-touch a");
-      $linkForTouch.attr("href", link);
-      $linkForTouch.text(link);
-      const range = window.document.createRange();
-      range.selectNode($linkForTouch[0]);
-      window.getSelection().addRange(range);
-    }
+    const linkInput = this.element.querySelector("#share-link input");
+    linkInput.value = this.link;
+
+    // Wait for the fade-in transition to finish before selecting the link:
+    window.setTimeout(() => {

Ember.run.later

[jjaffeux]

happy to see this code gone

[jjaffeux]

This is minor, but I feel like I have to tell this. Given you now use later with 200ms in didInsert, there’s a possibility that a transition will be called before 200ms, destroying the element and making this block fail. You can try to add a longer later, and do a transition to see if the consequences are minor (just an error logged) or more problematic, if yes, you should ensure the element has been found.

[pmusaraj]

Thanks @jjaffeux, the consequences were minor (only a console error logged) but I added the element check anyway.