Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix checkbox/radio shared editor on Mac #1459

Merged
merged 3 commits into from
Apr 27, 2020
Merged

Conversation

msssk
Copy link
Contributor

@msssk msssk commented Apr 25, 2020

Editor: wrap checkbox and radio button shared editors on Mac

In OS X form controls are not considered focusable elements. Some browsers
(Safari, Firefox) adopt this decision:
whatwg/html/issues/4356
https://bugzilla.mozilla.org/show_bug.cgi?id=1524863
https://bugs.webkit.org/show_bug.cgi?id=22261
The resulting implementation is broken - you can call focus() on a checkbox
and it will be focused, but clicking on a checkbox does not focus it. Further,
clicking on a focused checkbox blurs it. To overcome these challenges
checkboxes and radios are wrapped in a div that can receive focus (and blur)
in a consistent manner. Without the wrapper a valid blur is indistinguishable from an invalid blur. With the wrapper the blur event's target and relatedTarget enable reliably identifying invalid blurs and ignoring them.

Notes

  • This PR changes the way Dijit widget events are monitored - instead of using widget events, the DOM events are used directly. Widget events are not useful in this situation since they pass no event object; event.target and event.relatedTarget are crucial for this solution. In testing with dijit/form/CheckBox and dijit/form/RadioButton functionality remains the same.
    • dgrid/Editor.js

      Line 276 in 3a7416f

      focusNode.focus();
      • Previously, cmp.focus() would either call HTMLInputElement#focus or a widget's focus method, with this PR the widget's focusNode is used, so HTMLInputElement#focus is always called
    • dgrid/Editor.js

      Line 766 in 3a7416f

      (column._editorBlurHandle = on.pausable(this._getEditorFocusNode(cmp), 'blur', onblur)).pause();
      • Previously, on.pausable(cmp, 'blur', onblur) would listen for a widget's blur event if cmp was a widget. With this PR a listener for blur is registered directly on the widget's focusNode.
  • As of April 2020, out of testing Chrome, Firefox, and Safari on OS X, the broken behavior has only been observed in Firefox and Safari. However, at the core the problem is an issue with OS X's decision on which form controls are focusable elements, so the fix is being applied if has('mac') is true (1, 2) without further detection of which browser. The fix is unnecessary in unaffected browsers, but in testing works fine.

Fixes #1458

In OS X form controls are not considered focusable elements. Some browsers
(Safari, Firefox) adopt this decision:
whatwg/html#4356
https://bugzilla.mozilla.org/show_bug.cgi?id=1524863
https://bugs.webkit.org/show_bug.cgi?id=22261
The resulting implementation is broken - you can call `focus()` on a checkbox
and it will be focused, but clicking on a checkbox does not focus it. Further,
clicking on a focused checkbox blurs it. To overcome these challenges
checkboxes and radios are wrapped in a div that can receive focus (and blur)
in a consistent manner.

Fixes SitePen#1458
Simplify handling of native checkbox and radio inputs
@msssk msssk requested a review from maier49 April 25, 2020 07:21
@msssk msssk merged commit d8d2a16 into SitePen:master Apr 27, 2020
@msssk msssk deleted the 1458-mac-checkbox branch April 27, 2020 20:37
msssk added a commit that referenced this pull request Apr 28, 2020
Editor: wrap checkbox and radio button shared editors on Mac

In OS X form controls are not considered focusable elements. Some browsers
(Safari, Firefox) adopt this decision:
whatwg/html#4356
https://bugzilla.mozilla.org/show_bug.cgi?id=1524863
https://bugs.webkit.org/show_bug.cgi?id=22261
The resulting implementation is broken - you can call `focus()` on a checkbox
and it will be focused, but clicking on a checkbox does not focus it. Further,
clicking on a focused checkbox blurs it. To overcome these challenges
checkboxes and radios are wrapped in a div that can receive focus (and blur)
in a consistent manner.

Fixes #1458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CheckBox Editor does not work as expected in Safari
2 participants