Fixes #150.

[dahjelle]

Since the <li>'s onMouseUp handler will set optionsOpen to false, we don't need to do that here if we click on the <li>. Fixes #150.

[dahjelle]

And…I haven't the slightest idea why those tests are failing. They fail locally (forgot to look earlier…oops!), but they are also failing on master. Perhaps a dependency upgraded?

[asiandrummer]

@dahjelle - Thanks for fixing this! I'll take a look at the master failure in a bit.

[asiandrummer]

A small nit, but upEvent.target.parentNode.parentNode could be named into something more readable

[dahjelle]

I tried a different tactic using compareDocumentPosition, but I'm not necessarily convinced it is more readable. It is probably less fragile. Thoughts?

[asiandrummer]

How about:

const isOptionsMenuClicked = !(
  downTarget.parentNode.compareDocumentPosition(upEvent.target) &
  Node.DOCUMENT_POSITION_CONTAINED_BY
);
if (isOptionsMenuClicked) {

?

[asiandrummer]

So the build fails on react-test-utils@15.2.1 because the DOM provided to CodeMirror doesn't have getClientRects function while creating a Range object. I'm trying to see if this is a known issue - nonetheless, I'll get the tests working soon. Thanks for your patience!

[dahjelle]

Thanks for your patience!

Thank you for the hand-holding! Glad to help. :-D

How about…

Yes, I like it. Thanks! I moved the ! to make the variable name make more sense. It still isn't strictly accurate—it's something more like isOptionsMenuClickedAndNotAlreadyClosed—but I think it's close enough to read the code and get an idea of what is going on.

So the build fails on react-test-utils@15.2.1 because the DOM provided to CodeMirror doesn't have getClientRects function while creating a Range object. I'm trying to see if this is a known issue…

It looks like js-dom's implementation of Range is a work-in-progress.

For whatever it's worth, I found this page where they had a suggestion to add a mocked document.createRange to the tests. If I add

document.createRange = function () {
  return {
    setEnd() { },
    setStart() { },
    getBoundingClientRect() {
      return { right: 0 };
    },
    getClientRects() {
      return { right: 0 };
    }
  };
};

to GraphiQL-test.js, the tests pass.

[asiandrummer]

omg that's exactly what we needed - 010d47 takes care of this!

[dahjelle]

Cool! I rebased this PR on master. :-D