Fix premature close with content placed over trigger #1335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@cibernox let me know if you have any feedback or thoughts on this. Also happy to split out the test fixes into a separate PR if you prefer that. |
|
Also note there are some unrelated test failures for some ember-try build scenarios. These started occurring recently in other PRs as well, for example: https://travis-ci.org/cibernox/ember-power-select/jobs/653542073?utm_medium=notification&utm_source=github_status EDIT/UPDATE: have posted this PR cibernox/ember-basic-dropdown#534 to address test/build failures in Ember Basic Dropdown. If that PR gets merged and we tag a new release, we should be able to get the tests passing here as well. UPDATE 2: this PR has been rebased with Ember Basic Dropdown update, and tests are now passing on Travis, though success status is not being reported yet here. @cibernox thoughts on merging this? |
### Related issues - Addresses #1334 - Addresses #1282 - Addresses #1263 ### Background Using a value of `mousedown` for the `eventType` that is set on the Ember Basic Dropdown trigger is problematic in some cases, especially when content overlaps the trigger. The reason for this is that the subsequent `mouseup` event handler that is set in the `addHandlers` action in the `options.ts` file is triggered immediately after the `mousedown` that opens the dropdown content in the first place. The `mouseup` event handler calls `findOptionAndPerform` which will select any valid, (though in this case accidentally) highlighted option and then immediate close the dropdown when it makes the selection. Note that even if the above `mouseup` event listener is bypassed, Ember Basic Dropdown _will still immediately close_ the dropdown, though this time without Power Select having made a selection. This happens because, in the `basic-dropdown-content.ts` file, in the `setup` action adds a `click` event listener (based on the value of `this.args.rootEventType`) to the document, to ensure the content is closed when the user clicks away from the menu.... but when the resulting `handleRootMouseDown` function is called the `click` event target is `body`, which signals EBD to close the content. It's not clear to me why the `target` is `body` in this function, but I suspect it may be related to the `mousedown` and `mouseup` targets differing here, when the content appears above the trigger. Specifying `@eventType='click'` instead bypasses both problems because `click` is fired _only after_ the `mousedown` and `mouseup` events, resulting in more predictable behavior, regardless of location of the content in relation to the trigger. ### Other Notes - Also fixes unrelated, intermittent test failures ### Related, alternative PR - #1333
erikap
added a commit
to erikap/ember-power-select
that referenced
this pull request
Apr 12, 2020
Copied from cibernox#1335 but applied on ember-power-select v3.x
erikap
added a commit
to rollvolet/frontend-crm
that referenced
this pull request
Apr 12, 2020
Applies fix of cibernox/ember-power-select#1335 on ember-power-select 3.x
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issues
Background
Using a value of
mousedownfor theeventTypethat is set on the Ember Basic Dropdown trigger is problematic in some cases, especially when content overlaps the trigger. The reason for this is that the subsequentmouseupevent handler that is set in theaddHandlersaction in theoptions.tsfile is triggered immediately after themousedownthat opens the dropdown content in the first place. Themouseupevent handler callsfindOptionAndPerformwhich will select any valid, (though in this case accidentally) highlighted option and then immediate close the dropdown when it makes the selection.Note that even if the above
mouseupevent listener is bypassed, Ember Basic Dropdown will still immediately close the dropdown, though this time without Power Select having made a selection. This happens because, in thebasic-dropdown-content.tsfile, in thesetupaction adds aclickevent listener (based on the value ofthis.args.rootEventType) to the document, to ensure the content is closed when the user clicks away from the menu.... but when the resultinghandleRootMouseDownfunction is called theclickevent target isbody, which signals EBD to close the content. It's not clear to me why thetargetisbodyin this function, but I suspect it may be related to themousedownandmouseuptargets differing here, when the content appears above the trigger.Specifying
@eventType='click'instead bypasses both problems becauseclickis fired only after themousedownandmouseupevents, resulting in more predictable behavior, regardless of location of the content in relation to the trigger.Other Notes
Related, alternative PR