[url_launcher] Added draggable property [#102735] #5430

kristoffer-zliide · 2022-04-28T12:13:34Z

Allowing use of the Link widget in lists with mouse drag enabled.

In a desktop web browser, links are be default draggable. If they are put in e.g. a list, then that list cannot be scrolled by dragging it with the mouse. This PR makes it possible to disable this behavior.

Issues fixed: #102735

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

allowing use of the Link widget in lists with mouse drag enabled.

google-cla · 2022-04-28T12:13:37Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

stuartmorgan-g · 2022-05-12T20:29:53Z

@ditman Ping on this review?

ditman · 2022-05-21T00:46:12Z

Apologies! I'm going to bump this to @mdebbar, he knows the Link widget much better than I do.

Mouad, do you mind taking a look? Thanks!!

stuartmorgan-g · 2022-06-09T20:36:00Z

From triage: @mdebbar Ping on this review.

mdebbar

The PR looks good to me. I would like for @ditman to take a look at the pubspec changes since I'm not very familiar with how plugin dependencies are supposed to be.

mdebbar · 2022-06-10T16:04:01Z

packages/url_launcher/url_launcher_web/example/pubspec.yaml

+  url_launcher:
+    path: ../../url_launcher/
+dependency_overrides:
+  url_launcher_web:
+    path: ../
+  url_launcher_platform_interface:
+    path: ../../url_launcher_platform_interface/


@ditman I'm not qualified to review this part of the PR. Could you take a look?

This will need to be reverted so we can merge and publish the affected packages in the correct order.

ditman

We cannot merge this PR as is, because we now need to update version and CHANGELOG of the affected packages. Could you prepare a separate PR for each of the packages, so we can merge and publish them in order?

ditman · 2022-06-10T18:06:56Z

packages/url_launcher/url_launcher_web/lib/src/link.dart

+      _element.style
+        // Chrome
+        ..userDrag = 'none'
+        // Edge
+        ..setProperty('-webkit-user-drag', 'none');
+      // Firefox
+      _element.draggable = false;
+      _dragSubscription = _element.onDragStart.listen((event) {
+        event.preventDefault();
+      });


I was checking caniuse.com, and it seems that:

there's no unprefixed user-drag property. Docs.

wouldn't draggable = false be enough?

Also, if disabling drag in the element, is it necessary to stop the default drag event?

I think this method could be much simplified to be just _element.draggable = false?

It seems you're close to being right. We started out with the styling approach since it allowed us to apply a global work-around in index.html (<style>a { -webkit-user-drag: none; }</style>). Probably just added user-drag since it was right there in html_dart2js.dart, and if it works, it's nicer than the -webkit- style. But it also seems that draggable works on both Chrome and Edge; however, I couldn't make it work on Firefox without preventDefault. In the end, I guess it's a matter of taste: Do you want style or properties for this? Do you want to wrap the drag subscription in some Firefox shim?

After some tests, it seems that the only solution that seems to work the same across all platforms is preventDefault on the dragstart event. See:

https://jsfiddle.net/ditman/2kLuo8vd

If draggable=false, none of the browsers will trigger a dragstart event (as expected).

In Firefox, draggable=false works (it won't let you drag the Link), but it'll also start selecting from the text of the link as the user drags (which I guess is what you couldn't get to work?). This is a firefox quirk, but I couldn't find anything relevant in their bug tracker.

(My second favorite version might be draggable=false and if the browser is firefox, prevent the selectstart event (draggable=false + no selectstart in the JSFiddle), which doesn't interfere with the selection from the outside as much as user-select in CSS does!)

Actually, what we couldn't get to work is the issue I linked to: links in a list. For that case preventing select start doesn't seem to be sufficient. But interactions with text selection should indeed probably also be considered.

ditman · 2022-06-10T18:07:30Z

packages/url_launcher/url_launcher_web/example/pubspec.yaml

+  url_launcher:
+    path: ../../url_launcher/
+dependency_overrides:
+  url_launcher_web:
+    path: ../
+  url_launcher_platform_interface:
+    path: ../../url_launcher_platform_interface/


This will need to be reverted so we can merge and publish the affected packages in the correct order.

stuartmorgan-g · 2022-06-13T18:13:14Z

Could you prepare a separate PR for each of the packages, so we can merge and publish them in order?

We actually just do this one at a time; see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins

So the next step is to make just the platform interface PR, then once that lands, the implemenation package PR, then finally rebase this one.

stuartmorgan-g · 2022-07-07T20:33:06Z

@kristoffer-zliide Are you planning on splitting out the platform interface PR, as described above?

kristoffer-zliide · 2022-07-14T09:18:38Z

@stuartmorgan: Yeah, but I'm not sure if we've reached a conclusion on this: https://github.com/flutter/plugins/pull/5430/files#r896078916. I can make pull requests that adds draggable and preventDefault (ie, what's done for Firefox here: https://github.com/flutter/plugins/pull/5430/files#diff-77a04b3b00b00a4480b7d5e75e089dde7cca99ccebcc4f43e0e25d2bba5346e0R255), then you can always tweak it to your heart's content afterwards?

stuartmorgan-g · 2022-07-15T15:49:36Z

@ditman You marked this as approved; what was your take on the resolution of that comment thread?

ditman · 2022-07-15T18:02:09Z

@ditman You marked this as approved; what was your take on the resolution of that comment thread?

@stuartmorgan let's go with the solution as implemented. I did some testing and indeed for cross-browser support, you need more than one approach, as @kristoffer-zliide correctly pointed out.

Apologies I wasn't more clear in that regard!

stuartmorgan-g · 2022-08-16T16:06:33Z

@kristoffer-zliide It sounds from the above like this was ready for a platform interface PR; are you still planning on splitting that out to move this forward?

kristoffer-zliide · 2022-08-17T09:23:25Z

@stuartmorgan: here you go

stuartmorgan-g · 2022-09-13T17:52:26Z

Status update from triage: blocked on resolving the breaking change issue in #6282.

stuartmorgan-g · 2022-09-29T20:21:22Z

Status from triage: still blocked on #6282

stuartmorgan-g · 2022-10-27T20:16:19Z

Since this is still blocked on the other PR, I'm going to go ahead and mark this as a draft just to avoid having it come up in our triage queue. Once the blocking issue is resolved, this can be marked as ready for review again.

stuartmorgan-g · 2023-01-12T21:25:44Z

Closing since #6282 is now closed.

[url_launcher] Added draggable property [#102735]

b4499bf

allowing use of the Link widget in lists with mouse drag enabled.

kristoffer-zliide requested a review from ditman as a code owner April 28, 2022 12:13

github-actions bot added p: url_launcher platform-web labels Apr 28, 2022

kristoffer-zliide mentioned this pull request Apr 28, 2022

[Proposal][url_launcher][web] Allow draggable behaviour of Link to be configurable. flutter/flutter#102735

Open

Cancel subscription on dispose.

50549f6

ditman requested a review from mdebbar May 21, 2022 00:46

mdebbar approved these changes Jun 10, 2022

View reviewed changes

ditman approved these changes Jun 10, 2022

View reviewed changes

kristoffer-zliide mentioned this pull request Aug 17, 2022

[url_launcher] Added draggable property to link info. #6282

Closed

11 tasks

stuartmorgan-g marked this pull request as draft October 27, 2022 20:16

stuartmorgan-g closed this Jan 12, 2023

[url_launcher] Added draggable property [#102735] #5430

[url_launcher] Added draggable property [#102735] #5430

Uh oh!

Conversation

kristoffer-zliide commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

google-cla bot commented Apr 28, 2022

Uh oh!

stuartmorgan-g commented May 12, 2022

Uh oh!

ditman commented May 21, 2022

Uh oh!

stuartmorgan-g commented Jun 9, 2022

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

mdebbar Jun 10, 2022

Choose a reason for hiding this comment

Uh oh!

ditman Jun 10, 2022

Choose a reason for hiding this comment

Uh oh!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

ditman Jun 10, 2022

Choose a reason for hiding this comment

Uh oh!

kristoffer-zliide Jun 13, 2022

Choose a reason for hiding this comment

Uh oh!

ditman Jun 14, 2022 • 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!

ditman Jun 14, 2022 • 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!

kristoffer-zliide Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

ditman Jun 10, 2022

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Jun 13, 2022

Uh oh!

stuartmorgan-g commented Jul 7, 2022

Uh oh!

kristoffer-zliide commented Jul 14, 2022

Uh oh!

stuartmorgan-g commented Jul 15, 2022

Uh oh!

ditman commented Jul 15, 2022

Uh oh!

stuartmorgan-g commented Aug 16, 2022

Uh oh!

kristoffer-zliide commented Aug 17, 2022

Uh oh!

stuartmorgan-g commented Sep 13, 2022

Uh oh!

stuartmorgan-g commented Sep 29, 2022

Uh oh!

stuartmorgan-g commented Oct 27, 2022

Uh oh!

stuartmorgan-g commented Jan 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kristoffer-zliide commented Apr 28, 2022 •

edited

Loading

ditman Jun 14, 2022 •

edited

Loading

ditman Jun 14, 2022 •

edited

Loading