FEATURE: Select emojis in picker via keyboard (#18163)

* DEV: Make emoji elements focusable

Since emoji elements are of type `<img>` it requires a `tablindex="0"` in order to be focusable.

* WIP: Handle emoji focus/selection via arrow keys

Near completion, however, need a few fixes/improvements and overall code cleanup

* WIP: Testing

* DEV: Fixes and cleanup

* DEV: Follow conventions

* DEV: Improve up/down traversal when recents present

* DEV: Emoji markup in tests should include `tabindex`

* DEV: Add `tabindex` to topic tests

* DEV: Variable name as `searchInput` instead of `searchBar`

* DEV: Use appropriate method name (`_setNumEmojiPerRow`)

* DEV: Add comments and avoid nested if

* WIP: Adding test

* Fix first test

* DEV: Add assertions for arrow keys and escape key

* Some fixes for up/down navigation

This does not fix everything, when going from one section to another,
there are issues

* Fix a small regression

* FIX: Ability to focus on search results

Fixes regression

* Refactor calculating next up/down emoji

* Debugging test failure

* Skip stubborn CI test, add others

Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
This commit is contained in:
Keegan George 2022-09-21 13:21:36 -07:00 committed by GitHub
parent e3d7253199
commit eb987460f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 218 additions and 26 deletions

View File

@ -41,6 +41,10 @@ export default Component.extend({
usePopper: true,
placement: "auto", // one of popper.js' placements, see https://popper.js.org/docs/v2/constructors/#options
initialFilter: "",
elements: {
searchInput: ".emoji-picker-search-container input",
picker: ".emoji-picker-emoji-area",
},
init() {
this._super(...arguments);
@ -97,7 +101,6 @@ export default Component.extend({
if (!emojiPicker) {
return;
}
const popperAnchor = this._getPopperAnchor();
if (!this.site.isMobileDevice && this.usePopper && popperAnchor) {
@ -245,10 +248,100 @@ export default Component.extend({
@action
keydown(event) {
if (event.code === "Escape") {
const arrowKeys = ["ArrowDown", "ArrowUp", "ArrowLeft", "ArrowRight"];
const emojis = document.querySelectorAll(".emoji-picker-emoji-area .emoji");
let currentEmoji;
this.set(
"hoveredEmoji",
this._codeWithDiversity(event.target.title, this.selectedDiversity)
);
if (
event.key === "ArrowDown" &&
this._focusedOn(this.elements.searchInput)
) {
emojis[0].focus();
event.preventDefault();
return false;
}
if (event.key === "Escape") {
this.onClose(event);
return false;
}
if (arrowKeys.includes(event.key)) {
if (!this._focusedOn(this.elements.picker)) {
return;
}
Array.from(emojis).find((e, index) => {
currentEmoji = index;
return e.isEqualNode(event.target);
});
if (event.key === "ArrowRight") {
let nextEmoji = currentEmoji + 1;
if (nextEmoji < emojis.length) {
emojis[nextEmoji].focus();
} else if (nextEmoji >= emojis.length) {
emojis[0].focus();
}
}
if (event.key === "ArrowLeft") {
const previousEmoji = currentEmoji - 1;
if (currentEmoji > 0) {
emojis[previousEmoji].focus();
}
}
const active = emojis[currentEmoji];
if (event.key === "ArrowDown") {
// source: https://stackoverflow.com/a/49090383/349424
// look for same element type with
// - higher offsetTop
// - same offsetLeft
const emojiBelow = [...emojis]
.filter((c) => c.offsetTop > active.offsetTop)
.find((c) => c.offsetLeft === active.offsetLeft);
emojiBelow?.focus();
}
if (event.key === "ArrowUp") {
// look for same element type with
// - lower offsetTop
// - same offsetLeft
const emojiAbove = [...emojis]
.reverse()
.filter((c) => c.offsetTop < active.offsetTop)
.find((c) => c.offsetLeft === active.offsetLeft);
if (emojiAbove) {
emojiAbove.focus();
} else {
document.querySelector(this.elements.searchInput).focus();
}
}
event.preventDefault();
return false;
}
if (event.key === "Enter") {
if (!this._focusedOn(".emoji")) {
return;
}
this.onEmojiSelection(event);
this.onClose(event);
event.preventDefault();
return false;
}
},
@action
@ -256,6 +349,11 @@ export default Component.extend({
this._applyFilter(event.target.value);
},
_focusedOn(item) {
// returns the item currently being focused on
return document.activeElement.closest(item) ? document.activeElement : null;
},
_applyFilter(filter) {
const emojiPicker = document.querySelector(".emoji-picker");
const results = document.querySelector(".emoji-picker-emoji-area .results");
@ -287,7 +385,7 @@ export default Component.extend({
const escaped = emojiUnescape(`:${escapeExpression(code)}:`, {
lazy: true,
});
return htmlSafe(`<span>${escaped}</span>`);
return htmlSafe(escaped);
},
_codeWithDiversity(code, selectedDiversity) {

View File

@ -37,7 +37,7 @@
</div>
<div class="section-group">
{{#each this.recentEmojis as |emoji|}}
{{replace-emoji (concat ":" emoji ":") (hash lazy=true)}}
{{replace-emoji (concat ":" emoji ":") (hash lazy=true class="recent-emoji")}}
{{/each}}
</div>
</div>
@ -55,7 +55,9 @@
{{#if emojis.length}}
<div class="section-group">
{{#each emojis as |emoji|}}
<span>
<img title={{emoji.code}} width="20" height="20" loading="lazy" class="emoji" src={{emoji.src}}>
</span>
{{/each}}
</div>
{{/if}}

View File

@ -5,7 +5,7 @@ import {
query,
queryAll,
} from "discourse/tests/helpers/qunit-helpers";
import { click, fillIn, visit } from "@ember/test-helpers";
import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers";
import { test } from "qunit";
acceptance("EmojiPicker", function (needs) {
@ -179,4 +179,94 @@ acceptance("EmojiPicker", function (needs) {
"it stores diversity scale"
);
});
test("emoji can be selected with keyboard", async function (assert) {
const searchInput = ".emoji-picker-search-container input";
await visit("/t/internationalization-localization/280");
await click("#topic-footer-buttons .btn.create");
await click(".emoji.btn");
let emojis = document.querySelectorAll(
".emoji-picker-emoji-area img.emoji"
);
assert.strictEqual(
document.activeElement,
document.querySelector(searchInput),
"search input is focused by default"
);
await triggerKeyEvent(searchInput, "keydown", "ArrowDown");
assert.strictEqual(
document.activeElement,
emojis[0],
"ArrowDown from search focuses on the first emoji result"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowRight");
assert.strictEqual(
document.activeElement,
emojis[1],
"ArrowRight from first emoji focuses on the second emoji"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowLeft");
assert.strictEqual(
document.activeElement,
emojis[0],
"ArrowLeft from second emoji focuses on the first emoji"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowRight");
await triggerKeyEvent(document.activeElement, "keydown", "Enter");
assert.strictEqual(
document.querySelector(".d-editor-input").value,
":smiley:",
"Pressing enter inserts the emoji markup in the composer"
);
await click("#topic-footer-buttons .btn.create");
await click(".emoji.btn");
await triggerKeyEvent(searchInput, "keydown", "ArrowDown");
emojis = document.querySelectorAll(".emoji-picker-emoji-area img.emoji");
assert.strictEqual(
document.activeElement,
document.querySelector(".emoji-picker-emoji-area .emoji.recent-emoji"),
"ArrowDown focuses on the first emoji result (recent emoji)"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowDown");
assert.strictEqual(
document.activeElement,
document.querySelector(".emojis-container .emoji[title='grinning']"),
"ArrowDown again focuses on the first emoji result in a section"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowRight");
await triggerKeyEvent(document.activeElement, "keydown", "ArrowRight");
await triggerKeyEvent(document.activeElement, "keydown", "ArrowRight");
assert.strictEqual(
document.activeElement,
emojis[4],
"ArrowRight moves focus to next right element"
);
await triggerKeyEvent(document.activeElement, "keydown", "ArrowUp");
assert.strictEqual(
document.activeElement,
document.querySelector(searchInput),
"ArrowUp from first row items moves focus to input"
);
});
test("emoji picker can be dismissed with escape key", async function (assert) {
await visit("/t/internationalization-localization/280");
await click("#topic-footer-buttons .btn.create");
await click("button.emoji.btn");
await triggerKeyEvent(document.activeElement, "keydown", "Escape");
assert.notOk(exists(".emoji-picker"));
});
});

View File

@ -32,12 +32,12 @@ discourseModule("Unit | Utility | emoji", function () {
);
testUnescape(
"emoticons :)",
`emoticons <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
`emoticons <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji' tabindex='0'>`,
"emoticons are still supported"
);
testUnescape(
"With emoji :O: :frog: :smile:",
`With emoji <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/o.png?v=${v}' title='O' alt='O' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/frog.png?v=${v}' title='frog' alt='frog' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
`With emoji <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/o.png?v=${v}' title='O' alt='O' class='emoji' tabindex='0'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/frog.png?v=${v}' title='frog' alt='frog' class='emoji' tabindex='0'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'>`,
"title with emoji"
);
testUnescape(
@ -47,27 +47,27 @@ discourseModule("Unit | Utility | emoji", function () {
);
testUnescape(
"(:frog:) :)",
`(<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/frog.png?v=${v}' title='frog' alt='frog' class='emoji'>) <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'>`,
`(<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/frog.png?v=${v}' title='frog' alt='frog' class='emoji' tabindex='0'>) <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji' tabindex='0'>`,
"non-word characters allowed next to emoji"
);
testUnescape(
":smile: hi",
`<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> hi`,
`<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'> hi`,
"start of line"
);
testUnescape(
"hi :smile:",
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'>`,
"end of line"
);
testUnescape(
"hi :blonde_woman:t4:",
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'>`,
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji' tabindex='0'>`,
"support for skin tones"
);
testUnescape(
"hi :blonde_woman:t4: :blonde_man:t6:",
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_man/6.png?v=${v}' title='blonde_man:t6' alt='blonde_man:t6' class='emoji'>`,
`hi <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_woman/4.png?v=${v}' title='blonde_woman:t4' alt='blonde_woman:t4' class='emoji' tabindex='0'> <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blonde_man/6.png?v=${v}' title='blonde_man:t6' alt='blonde_man:t6' class='emoji' tabindex='0'>`,
"support for multiple skin tones"
);
testUnescape(
@ -95,7 +95,7 @@ discourseModule("Unit | Utility | emoji", function () {
);
testUnescape(
"Hello 😊 World",
`Hello <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blush.png?v=${v}' title='blush' alt='blush' class='emoji'> World`,
`Hello <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blush.png?v=${v}' title='blush' alt='blush' class='emoji' tabindex='0'> World`,
"emoji from Unicode emoji"
);
testUnescape(
@ -108,7 +108,7 @@ discourseModule("Unit | Utility | emoji", function () {
);
testUnescape(
"Hello😊World",
`Hello<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blush.png?v=${v}' title='blush' alt='blush' class='emoji'>World`,
`Hello<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/blush.png?v=${v}' title='blush' alt='blush' class='emoji' tabindex='0'>World`,
"emoji from Unicode emoji when inline translation enabled",
{
enable_inline_emoji_translation: true,
@ -124,7 +124,7 @@ discourseModule("Unit | Utility | emoji", function () {
);
testUnescape(
"hi:smile:",
`hi<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
`hi<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'>`,
"emoji when inline translation enabled",
{ enable_inline_emoji_translation: true }
);

View File

@ -143,7 +143,7 @@ discourseModule("Unit | Model | topic", function () {
assert.strictEqual(
topic.get("fancyTitle"),
`<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'> with all <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji'> the emojis <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/pear.png?v=${v}' title='pear' alt='pear' class='emoji'><img width=\"20\" height=\"20\" src='/images/emoji/google_classic/peach.png?v=${v}' title='peach' alt='peach' class='emoji'>`,
`<img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'> with all <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/slight_smile.png?v=${v}' title='slight_smile' alt='slight_smile' class='emoji' tabindex='0'> the emojis <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/pear.png?v=${v}' title='pear' alt='pear' class='emoji' tabindex='0'><img width=\"20\" height=\"20\" src='/images/emoji/google_classic/peach.png?v=${v}' title='peach' alt='peach' class='emoji' tabindex='0'>`,
"supports emojis"
);
});
@ -173,7 +173,7 @@ discourseModule("Unit | Model | topic", function () {
assert.strictEqual(
topic.get("escapedExcerpt"),
`This is a test topic <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji'>`,
`This is a test topic <img width=\"20\" height=\"20\" src='/images/emoji/google_classic/smile.png?v=${v}' title='smile' alt='smile' class='emoji' tabindex='0'>`,
"supports emojis"
);
});

View File

@ -98,7 +98,7 @@ export function performEmojiUnescape(string, opts) {
opts.skipTitle ? "" : `title='${title}'`
} ${
opts.lazy ? "loading='lazy' " : ""
}alt='${title}' class='${classes}'>`
}alt='${title}' class='${classes}' tabindex='0'>`
: m;
};

View File

@ -87,12 +87,18 @@ sup img.emoji {
.section-group,
.results {
display: flex;
flex-wrap: wrap;
img.emoji {
padding: 0.25em;
padding: 0.25em 0.28em;
cursor: pointer;
margin: 0;
display: inline-flex;
&:hover {
background: $tertiary-low;
&:hover,
&:focus {
background: var(--tertiary-low);
border-radius: 3px;
}
}
}
@ -103,10 +109,6 @@ sup img.emoji {
&:empty {
display: none;
}
img.emoji {
padding: 0.5em;
}
}
}