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

[EuiDraggable] Dragging doesnt work for EuiAccordion draggable on mobile with disableInteractiveElementBlocking #5903

Closed
flash1293 opened this issue May 17, 2022 · 4 comments · Fixed by #6031
Assignees
Labels

Comments

@flash1293
Copy link
Contributor

Related to elastic/kibana#128098

If EuiDraggable is used with an EuiAccordion child, using the button of an accordion as drag handle (which requires to set disableInteractiveElementBlocking), then the drag is aborted right after it starts on touch devices.

Codesandbox to reproduce:

https://codesandbox.io/s/interesting-kalam-z03t4l?file=/demo.js:1151-1184

  • Open preview in codesandbox in new window
  • View with iPad Mini via dev tools
Kapture.2022-05-17.at.17.03.36.mp4

Without disableInteractiveElementBlocking a button can't be the drag handle.

cc @chandlerprall

@flash1293 flash1293 added bug ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels May 17, 2022
@chandlerprall
Copy link
Contributor

I am unable to reproduce the issue in the latest chrome (v103.0.5060.53) with the provided codesandbox. Tried both mac & windows with various screen sizes emulated in both horizontal & vertical orientations.

@chandlerprall
Copy link
Contributor

chandlerprall commented Jun 27, 2022

Joe figured out the repro requirement - viewing the deployed code sandbox with responsive+ipad mini enabled, at 75% zoom.

Was able to reproduce by moving the accordion example into EUI docs and viewing with the same env as above, react-beautiful-dnd's operation is being interrupted by a global error: ResizeObserver loop limit exceeded 🤦‍♂️

Should be a fun one.

@chandlerprall chandlerprall self-assigned this Jun 27, 2022
@chandlerprall chandlerprall removed the ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible label Jun 27, 2022
@chandlerprall
Copy link
Contributor

chandlerprall commented Jun 27, 2022

First thought was to patch react-beautiful-dnd's ErrorBoundary component's onWindowError handler, but their transpilation step converts that code to the old pseudo-classes style:

function ErrorBoundary() {
  var _this;
  _this = _React$Component.call.apply(_React$Component, [this].concat(args)) || this;

  _this.onWindowError = function (event) {

closing over _this and preventing any access, unless we also patched relevant parts of react to intercept element creation 😅

So I'm left with three ideas, will discuss at the team's sync meeting tomorrow

  1. Because the error boundary code adds a global event handler on window on mount, we can beat it by adding our own window error handler that stopsImmediatePropagation for ResizeObserver loop limit exceeded errors. I have tested this as a util method an app could call once at startup:
export function patchResizeObserverLoopLimitExceeded() {
  window.addEventListener('error', (e) => {
    if (e.message.includes('ResizeObserver loop limit exceeded')) {
      e.stopImmediatePropagation();
    }
  });
}
  1. Deactivate EUI's resize observers when a drag operation is started. This could be done automatically by attaching global event handlers or by exposing pause/unpause utility methods for EUI components & applications to use.

  2. Intercept react-beautiful-dnd's event listener when it's added to window and don't call it for resize loop errors

const _addEventListener = window.addEventListener;
window.addEventListener = (event, listener, ...opts: any[]) => {
  if (event === 'error') {
    // check if this listener is react-beautiful-dnd's onWindowError handler
    const listenerStr = listener.toString();
    if (
      listenerStr.includes('var callbacks = _this.getCallbacks();') &&
      listenerStr.includes('if (callbacks.isDragging()) {')
    ) {
      const _listener = listener;
      listener = (event) => {
        if (event.message.includes('ResizeObserver loop limit exceeded')) {
          // don't alert the listener
        } else {
          _listener(event);
        }
      };
    }
  }
  _addEventListener.call(window, event, listener, ...opts);
};

@chandlerprall
Copy link
Contributor

chandlerprall commented Jul 5, 2022

Forgot to follow up after last week's sync: I'm exploring two ideas for handling this in the eui drag&drop component(s).

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

Successfully merging a pull request may close this issue.

2 participants