You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/07/05 06:51:49 UTC
[GitHub] [beam] alexeyinkin opened a new pull request, #22154: Defocus iframe on blur or mouseout (#22153)
alexeyinkin opened a new pull request, #22154:
URL: https://github.com/apache/beam/pull/22154
Fixes #22153
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1174945245
The workaround:
If not in iframe, do nothing.
If Safari, do nothing because it is not affected.
If in Firefox, programmatically release focus on iframe's `blur` event. In Firefox it is sufficient. The downside is that selection is lost on clicking outside.
Otherwise programmatically release focus on iframe's `mouseout` event. In Chrome the solution for Firefox is insufficient. On the first outside click, it still scrolls back to the iframe, and only the second click is free to leave the iframe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on a diff in pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #22154:
URL: https://github.com/apache/beam/pull/22154#discussion_r913459429
##########
playground/frontend/lib/main.dart:
##########
@@ -17,12 +17,14 @@
*/
import 'package:flutter/material.dart';
+import 'package:flutter_issue_106664_workaround/flutter_issue_106664_workaround.dart';
Review Comment:
The package code: https://github.com/alexeyinkin/flutter-issue-106664-workaround/blob/main/lib/src/web.dart
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1183729060
I'm thinking it might make sense to eventually separate the Beam Playground into its own repository to isolate these issues from the main repo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on a diff in pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #22154:
URL: https://github.com/apache/beam/pull/22154#discussion_r918658578
##########
playground/frontend/pubspec.yaml:
##########
@@ -44,6 +44,7 @@ environment:
# the latest version available on pub.dev. To see which dependencies have newer
# versions available, run `flutter pub outdated`.
dependencies:
+ akvelon_flutter_issue_106664_workaround: ^0.1.1
Review Comment:
This is how I compare these options:
## Local package in Beam repo, referenced in pubspec.yaml by relative path ##
++ **Totally under Apache's control.**
-- **Low chance that anyone will update the code** if the behavior of the bug changes in some Flutter versions or browsers since people will just forget about the issue.
## In-project code ##
Same as above, but
-- **Even less chance anyone will update the code** since there is even no hint in pubspec.yaml that this is a issue.
-- **The import of `dart:html` produces a linter warning** ("Avoid web libraries in non-web packages") that has to be silenced. This is one of the reasons I like outsourcing it to dedicated packages so the main code is platform-agnostic and warning-free.
-- **Cannot be reused** in ToB or other projects even within the same Beam repository.
## Git repo, referenced by URL in pubspec.yaml ##
++ **Can be reused** in ToB (should it go in Flutter) or even other apps outside of Beam repository.
++ **Higher chance of receiving updates** since it is a separate unit at Akvelon.
-- **The least robust way** from Apache's perspective since the repository can be removed, or hidden, or the branch can be force-pushed into a broken state.
## Pub.dev package ##
All benefits of the above, and
++ **Package will always be there** since pub.dev does not allow to change or delete what was published, unlike git.
++ **Highest chance of receiving updates** since 3rd parties will adopt this too.
++ **Notification when the bug is fixed.** Pub.dev packages can be flagged as discontinued, and this will produce a warning on Playground build. It will hint the Playground maintainers to remove the package which will improve user experience (no selection losses on mouse out).
So pub.dev package is not necessary, but I see it as the most beneficial way to all parties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #22154:
URL: https://github.com/apache/beam/pull/22154#discussion_r918356904
##########
playground/frontend/pubspec.yaml:
##########
@@ -44,6 +44,7 @@ environment:
# the latest version available on pub.dev. To see which dependencies have newer
# versions available, run `flutter pub outdated`.
dependencies:
+ akvelon_flutter_issue_106664_workaround: ^0.1.1
Review Comment:
Instead of publishing a package on pub.dev, could you create a local one and reference? One could also reference a depedency via local or GitHub references: https://dart.dev/tools/pub/dependencies. If a pub package is absolutely needed, I noticed two issues flagged in the scores: https://pub.dev/packages/akvelon_flutter_issue_106664_workaround/score
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1183728297
fair enough. I'll merge this for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1180662743
I also took this chance to rename `enabled` param to `editable`. `enabled` was vague, it gives an impression that disabled playground is unable to run the code. `editable` is more precise.
I did it in this branch because it is likely the fastest way to deploy it, and we want this change before URLs start to spread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1174686419
R: @damondouglas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem merged pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
pabloem merged PR #22154:
URL: https://github.com/apache/beam/pull/22154
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1175219328
Please hold this PR. It uses my personal package. We have decided to re-publish it at Akvelon's pub.dev account. We are busy doing this now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on a diff in pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on code in PR #22154:
URL: https://github.com/apache/beam/pull/22154#discussion_r918678514
##########
playground/frontend/pubspec.yaml:
##########
@@ -44,6 +44,7 @@ environment:
# the latest version available on pub.dev. To see which dependencies have newer
# versions available, run `flutter pub outdated`.
dependencies:
+ akvelon_flutter_issue_106664_workaround: ^0.1.1
Review Comment:
I have updated the package. It now have the max score, and the source is public here:
https://github.com/akvelon/flutter-issue-106664-workaround
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] alexeyinkin commented on pull request #22154: Defocus iframe on blur or mouseout (#22153)
Posted by GitBox <gi...@apache.org>.
alexeyinkin commented on PR #22154:
URL: https://github.com/apache/beam/pull/22154#issuecomment-1175884305
Before:
After:
(Note that we lose selection when moving mouse pointer out from the iframe.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org