markitondemand / mk-ui

A suite of JavaScript component libraries focused on accessible solutions to common UI challenges.
MIT License
9 stars 7 forks source link

Tooltip not working in IE9, and other issues. #24

Open lafsar opened 7 years ago

lafsar commented 7 years ago

Hello,

Using mk-ui v2.0.41.

Issue#1: Tooltips not working in IE9 We have a client that requires IE9 support (d'oh), and I'm seeing this error thrown everytime a tooltip is triggered, but no tooltip is actually displayed:

Unable to get property 'contains' of undefined or null reference File: core.js, Line: 1072, Column: 13

Seems like the this.el is not providing a valid element in the hasClass function.

Issue#2: The capture option is not exposed in the API. core.js has $.events which forces mouseenter and mouseleave to have capture mode enabled. This causes a problem when the trigger element has other nested elements (like an svg icon that is inside a button element), and I don't want to have duplicate events fired. Is it possible to expose an additional argument to both .on() and .one() that will allow the consumer to determine whether capture should be enabled regardless of event type?

Issue#3: _bindModalDown prevents the user from being able to select the tooltip contents since it monopolizes the click event on the modal. I needed to override this function in my Popover.js gist (see below). I'm not sure what side affects from disabling this function would be.

Issue#4: (design philosophy of Tooltip.js) It seems like it was intentional to have tooltips close on mouseleave (unless the modal is nested inside the trigger element, but this is undesirable in most cases), but I know of 3 clients that want tooltips to stay open on hover, and be able to select the contents with the mouse. It would be nice if this option was baked into Tooltip instead of having to extend it since this is a commonly expected feature. Also it would be a good idea to adjust the modal width if the height exceeds a certain amount (which can be configured in the options - see my gist and the comments)

I have a gist that extends Tooltip.js into Popover.js that implements this functionality, but it is a little hacky due to the fact that capture mode is not exposed in the api (See issue#2) and _bindModalDown is preventing mouse selection of the contents (See Issue#3). :

https://gist.github.com/lafsar/c8e8771ff94c767a5395c5cf11c0748f

micha3ldavid commented 7 years ago

Hi Luke,

Thanks for the detailed issue list. Here are some thoughts before moving forward.

  1. IE9 now goes in the bucket of "IE11" because of the "auto upgrade" Microsoft did a while back. Yes, users could have denied the update and that would result in the issue. The issue is that for class name manipulation we're using the DOM classList property which has API methods like add(), remove(), and contains(). This is what is causing your IE9 issue. In fact, this breaks ALL of Mk[ui] since the class manipulation code is in the Core. Changing this is debatable, since we'll be defaulting back to primitive class manipulation to meet the needs of a very limited audience.

  2. The events you bind via on() and one() are not actually bound to an event listener hence why there is no immediate DOM Event exposed. We could add the ability to supply the event handlers with a DOM Event where necessary, however, we would want to do this across the board for all components to stay consistent. This change is also considered a "breaking change" since any sites looking to upgrade their version of Mk[ui] would also need to edit their code to take the new parameter into account. That said, I think this could be a good change to implement.

  3. The reason you cannot select text is because we manipulate the mousedown event in order to catch/cancel blur/focus events. If you have a better way of doing this that does not include timers by all means let me know. I'll look into alternative ways of handling this as well when I get some free time.

  4. Yes, Tooltip was intentionally designed to close on mouseleave. This is the definition of a tooltip according to web usability standards. Think of a title attribute on a link. Once you add the ability to interact with the popup, it becomes a dialog. As part of web usability, dialogs should be triggered by click, not mouseover. Think of those mouseover/out navigation dropdowns back in the early 2000s. Really, really bad usability experience. Fast forward to 2017 and these navigation dropdowns are accessed by clicks/focus click/blur almost exclusively. That is the standard we built tooltip to follow and I'd recommend sticking with that. It's worth a conversation with your project manager and team about re-evaluating the usability of what you are creating. If their answers are still "no," we can of course look into alternative opt-in behavior to meet your needs.