-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
Get tests passing w/ Chrome's new selectionchange event behavior #1473
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
9c99e6f
Print the google-chrome version in CI
NullVoxPopuli 33a0619
Get FireFox tests passing 100% (previously, there was one failure)
NullVoxPopuli f59e273
Update click tests
NullVoxPopuli 05e067e
Update double click tests
NullVoxPopuli e8f7f68
Update blur tests
NullVoxPopuli d334d13
Update fillIn tests
NullVoxPopuli 907daf1
Update focus tests
NullVoxPopuli caaad9b
Update tab tests
NullVoxPopuli 525bc57
Update tap tests
NullVoxPopuli d300515
Update typeIn tests
NullVoxPopuli ce8cdcb
Chrome 127 has a bug w/ double selectionchange events, fixed in Chrom…
NullVoxPopuli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
export const isEdge = navigator.userAgent.indexOf('Edge') >= 0; | ||
|
||
// Unlike Chrome, Firefox emits `selectionchange` events. | ||
// Firefox emits `selectionchange` events. | ||
export const isFirefox = navigator.userAgent.indexOf('Firefox') >= 0; | ||
|
||
// Chrome emits `selectionchange` events. | ||
export const isChrome = navigator.userAgent.indexOf('Chrome') >= 0; |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { | |
insertElement, | ||
} from '../../helpers/events'; | ||
import hasEmberVersion from '@ember/test-helpers/has-ember-version'; | ||
import { isChrome } from '../../helpers/browser-detect'; | ||
import { | ||
registerHooks, | ||
unregisterHooks, | ||
|
@@ -202,6 +203,17 @@ module('DOM Helper: click', function (hooks) { | |
module('focusable element types', function () { | ||
let clickSteps = ['mousedown', 'focus', 'focusin', 'mouseup', 'click']; | ||
|
||
if (isChrome) { | ||
clickSteps = [ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'selectionchange', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just push There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then we'd need to add inverted conditions for safari and firefox |
||
]; | ||
} | ||
|
||
test('clicking a input via selector with context set', async function (assert) { | ||
element = buildInstrumentedElement('input'); | ||
|
||
|
@@ -297,7 +309,17 @@ module('DOM Helper: click', function (hooks) { | |
|
||
await click(child); | ||
|
||
assert.verifySteps(clickSteps); | ||
if (isChrome) { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
]); | ||
} else { | ||
assert.verifySteps(clickSteps); | ||
} | ||
assert.strictEqual( | ||
document.activeElement, | ||
element, | ||
|
@@ -334,15 +356,28 @@ module('DOM Helper: click', function (hooks) { | |
await click(focusableElement); | ||
await click(element); | ||
|
||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'blur', | ||
'focusout', | ||
]); | ||
if (isChrome) { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'selectionchange', | ||
'blur', | ||
'focusout', | ||
]); | ||
} else { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'blur', | ||
'focusout', | ||
]); | ||
} | ||
}); | ||
|
||
test('clicking on non-focusable element inside active element does not trigger blur on active element', async function (assert) { | ||
|
@@ -377,15 +412,28 @@ module('DOM Helper: click', function (hooks) { | |
await click(focusableElement); | ||
await click(element); | ||
|
||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'blur', | ||
'focusout', | ||
]); | ||
if (isChrome) { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'selectionchange', | ||
'blur', | ||
'focusout', | ||
]); | ||
} else { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'blur', | ||
'focusout', | ||
]); | ||
} | ||
}); | ||
|
||
test('clicking on non-focusable element does not trigger blur on non-focusable active element', async function (assert) { | ||
|
@@ -417,7 +465,24 @@ module('DOM Helper: click', function (hooks) { | |
await click(focusableElement); | ||
await click(element); | ||
|
||
assert.verifySteps(['mousedown', 'focus', 'focusin', 'mouseup', 'click']); | ||
if (isChrome) { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
'selectionchange', | ||
]); | ||
} else { | ||
assert.verifySteps([ | ||
'mousedown', | ||
'focus', | ||
'focusin', | ||
'mouseup', | ||
'click', | ||
]); | ||
} | ||
|
||
element.removeEventListener('mousedown', preventDefault); | ||
await click(element); | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we not do
selectionchange
for Firefox since that supports it also?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is slightly different.
When I was working on this PR, I had 5 browsers open
😅