plugins icon indicating copy to clipboard operation
plugins copied to clipboard

Published 20 hours ago • verified publisher icon flutter

[url_launcher] Added draggable property [#102735]

Open kristoffer-zliide opened this issue 2 years ago • 12 comments

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

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • [x] I signed the CLA.
  • [x] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • [x] I listed at least one issue that this PR fixes in the description above.
  • [ ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • [ ] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • [ ] I updated/added relevant documentation (doc comments with ///).
  • [ ] I added new tests to check the change I am making, or this PR is test-exempt.
  • [ ] All existing and new tests are passing.

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

kristoffer-zliide avatar Apr 28 '22 12:04 kristoffer-zliide

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.

google-cla[bot] avatar Apr 28 '22 12:04 google-cla[bot]

@ditman Ping on this review?

stuartmorgan avatar May 12 '22 20:05 stuartmorgan

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!!

ditman avatar May 21 '22 00:05 ditman

From triage: @mdebbar Ping on this review.

stuartmorgan avatar Jun 09 '22 20:06 stuartmorgan

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 avatar Jun 13 '22 18:06 stuartmorgan

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

stuartmorgan avatar Jul 07 '22 20:07 stuartmorgan

@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?

kristoffer-zliide avatar Jul 14 '22 09:07 kristoffer-zliide

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

stuartmorgan avatar Jul 15 '22 15:07 stuartmorgan

@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!

ditman avatar Jul 15 '22 18:07 ditman

@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?

stuartmorgan avatar Aug 16 '22 16:08 stuartmorgan

@stuartmorgan: here you go

kristoffer-zliide avatar Aug 17 '22 09:08 kristoffer-zliide

Status update from triage: blocked on resolving the breaking change issue in https://github.com/flutter/plugins/pull/6282.

stuartmorgan avatar Sep 13 '22 17:09 stuartmorgan

Status from triage: still blocked on https://github.com/flutter/plugins/pull/6282

stuartmorgan avatar Sep 29 '22 20:09 stuartmorgan

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 avatar Oct 27 '22 20:10 stuartmorgan

Closing since https://github.com/flutter/plugins/pull/6282 is now closed.

stuartmorgan avatar Jan 12 '23 21:01 stuartmorgan