join-the-flock / nest

Tools for hatching new Flutters
133 stars 6 forks source link

Custom Cursor Support for Web #3

Open indiealchemy opened 3 weeks ago

indiealchemy commented 3 weeks ago

Requesting investigation into Custom Cursor Support for web for Flock

Background:

The https://pub.dev/packages/custom_mouse_cursor package currently enables custom cursor support for Mac, Windows, and Linux. However there has remained a related outstanding feature request for years: Web support for custom cursors.

The package handles this feature request as proven by the web demo: https://timmaffett.github.io/custom_mouse_cursor/

The caveat is that Web cannot work without a PR merge to Flutter engine: https://github.com/flutter/engine/pull/41186 per the package's README.

Validity:

Creator of this package, @timmaffett has successfully had a previous PR merged into Flutter master to enable Window's support which speaks to the caliber of their work. They also have documented the origin of the changes and cite relevant PRs and issues within PR#41186.

PR #41186 Closure:

After serval communication cycles, the PR for web support seems to have fizzled due to "outstanding comments". See more here: https://github.com/timmaffett/custom_mouse_cursor/issues/2

jezell commented 3 weeks ago

@indiealchemy this is a great idea. My team could also benefit from custom cursor support on web.

timmaffett commented 3 weeks ago

The PR that I have for web support is super simple - and the hold up from the flutter team was quite frustrating - over semantics of the name of the arguments. https://github.com/flutter/engine/pull/41186

It basically allows all valid CSS types through in the 'kind' parameters - instead of disallowing 'image-set' and 'webkit-image-set' arbitrarily because someone didn't like how CSS supported this (I guess - I really could never figure out exactly what the issue was - I jumped through the hoops for a while - but ultimately I just gave up because I was only doing this PR to resolve issue backlogs and not because I wanted to spend endless time jumping through semantic hoops.

If you read through the conversation on that PR the issues being raised really makes no sense in the context of the PR itself, but how the web standards committee chose to support custom cursors in CSS.

The PR is REALLY the simplest way to implement this - not adding another layer over the web standards.

There are also tests for this PR included.

timmaffett commented 3 weeks ago

I just took the time to re-read the comments on https://github.com/flutter/engine/pull/41186 and I think that this PR is a perfect example of the frustrating handling of PRs by the team which has motivated the creation of the Flock fork.

Essentially @dkwingsmt is asking for an entirely new entry point to the platform layer that would simply then be forwarded to the activateSystemCursor entry point - because apparently he doesn't agree with CSS overloading the single 'cursor' property with system cursor names and with url(custom) cursors. I just didn't have the time to add the additional code necessary to add a entirely new entry point from platform independent to web platform. Not to mention that adding all this additional code (for a purely philosophical argument) just adds testing surface with 0 added benefit aside from semantics.

I was particularly surprised when @Hixie didn't just say that the simplest way was the best. (That was probably the first thing I had ever read from @Hixie that I did not agree with 😆). The argument 'kind' directly passes 'kinds' of cursors (with explicit names) that the underlying platform supports - and in this case we just allow an additional (valid) kind of password to be passed on to the underlying css cursor pathway. Adding a new intermediate pathway - to ultimately end up at the css cursor pathway with the css cursor kind argument - seems to contradict simple programming best practices of less code is better and KISS.

Still can't believe that PR was closed and not merged on the argument that we needed to add more code, complexity and testing surface.

matthew-carroll commented 3 weeks ago

Thanks for bringing your existing work to our attention. We should definitely take a look and see what we can do.

timmaffett commented 3 weeks ago

You have @indiealchemy to thank for bringing this to your attention - I only found this because of his @timmaffett reference.