Skip to content

Commit

Permalink
fix: changing Shortcuts.kts logic to use HashableKeys. (#1616)
Browse files Browse the repository at this point in the history
* As the ShortcutRegistration will always use HashableKeys, hence why we need to use it in our tests as well to not have any differences
between our tests and real-life application. (now we used simple Key objects, but we need to use HashableKeys)

* Cleaned up the code a bit to use the HashableKey creation only in the MockFilterJsonObject class.
(Based on @mcollovati  review, thank you)

* Put the getHashableKey() method into the class (mock JSON), so where we use it (suggestion by @mcollovati, thanks!).

* One more time, even nicer this way, like this one :) (Thanks again @mcollovati)

* Revert GridTester changes, and fix java doc for the newly added getHashableKey() method, fix imports.

* Using a bit more Kotlinish syntax for reflection (private inner class constructor), and being more strict on that we are using ShortcutRegistration class. (discussed with @mcollovati that this could be a better way)

* Enabling Shortcut related tests (were disabled by another dependency FIX pr to merge it to master)
  • Loading branch information
czp13 committed Mar 16, 2023
1 parent 390e53f commit baf3886
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

Expand All @@ -28,7 +27,6 @@
@ExtendWith(TreeOnFailureExtension.class)
class UIUnitShortcutTest extends UIUnitTest {

@Disabled
@Test
void fireShortcut_UIListener_invokedForExactMatch() {
AtomicInteger eventsCounter = new AtomicInteger();
Expand All @@ -52,7 +50,6 @@ void fireShortcut_UIListener_invokedForExactMatch() {
Assertions.assertEquals(2, eventsCounter.get());
}

@Disabled
@Test
void fireShortcut_nestedComponents_listenersInvoked() {
AtomicInteger buttonEvents = new AtomicInteger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ class AllTests : DynaTest({
searchSpecTest()
}

// group("shortcuts") {
// shortcutsTestBatch()
// }
group("shortcuts") {
shortcutsTestBatch()
}

test("Component.isTemplate does not fail without polymer templates dependency") {
expect(false) { Button("foo").isTemplate }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.vaadin.testbench.unit.internal

import com.vaadin.flow.component.Component
import com.vaadin.flow.component.Key
import com.vaadin.flow.component.KeyModifier
import com.vaadin.flow.component.ShortcutRegistration
import elemental.json.impl.JreJsonFactory
import elemental.json.impl.JreJsonObject
Expand All @@ -12,17 +11,30 @@ import elemental.json.impl.JreJsonObject
* If this stuff stops working, place a breakpoint into the [getBoolean]/[hasKey] function,
* to see what kind of keys you're receiving and whether it matches [filter].
*/
private class MockFilterJsonObject(val key: Key, val modifiers: Set<KeyModifier>) : JreJsonObject(JreJsonFactory()) {
private class MockFilterJsonObject(val key: Key, val modifiers: Set<Key>) : JreJsonObject(JreJsonFactory()) {
val filter: String
init {

// compute the filter
filter = mgenerateEventKeyFilter.invoke(null, key).toString() + " && " +
mgenerateEventModifierFilter.invoke(null, modifiers).toString()
// we call the private method with Hashable keys implementations as the ShortcutRegistration class is
// called by these objects/classes in regular cases.
mgenerateEventModifierFilter.invoke(null, modifiers.map { getHashableKey(it)}).toString()

// populate the json object so that KeyDownEvent can be created from it
put("event.key", key.keys.first())
}

/**
* Returns a HashableKey instance for the given key modifier.
* @param keyModifier the key modifier
* @return the HashableKey instance
*
*/
private fun getHashableKey(keyModifier: Key): Any? {
return chashableKey.newInstance(keyModifier)
}

override fun hasKey(key: String): Boolean {
// the "key" is a JavaScript expression which matches the key pressed.
// we need to match it against the 'filter'
Expand Down Expand Up @@ -54,9 +66,14 @@ private class MockFilterJsonObject(val key: Key, val modifiers: Set<KeyModifier>
companion object {
private val mgenerateEventKeyFilter = ShortcutRegistration::class.java.getDeclaredMethod("generateEventKeyFilter", Key::class.java)
private val mgenerateEventModifierFilter = ShortcutRegistration::class.java.getDeclaredMethod("generateEventModifierFilter", Collection::class.java)
private val chashableKey = ShortcutRegistration::class.java
.classLoader
.loadClass("com.vaadin.flow.component.ShortcutRegistration\$HashableKey")
.declaredConstructors[0]
init {
mgenerateEventKeyFilter.isAccessible = true
mgenerateEventModifierFilter.isAccessible = true
chashableKey.isAccessible = true
}
}
}
Expand Down Expand Up @@ -86,8 +103,7 @@ public fun Component._fireShortcut(key: Key, vararg modifiers: Key) {
// contains a boolean value with key 'filter'. We need to fake the json object
// as if it contained all filters (otherwise `matchesFilter()` would NPE on missing key)
// and respond true only to the matching filter.
@Suppress("UNCHECKED_CAST")
val data = MockFilterJsonObject(key, modifiers.toSet() as Set<KeyModifier>)
val data = MockFilterJsonObject(key, modifiers.toSet())

// the shortcut registration is only updated in [UI.beforeClientResponse]; run the registration code now.
MockVaadin.clientRoundtrip()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ class AllTests : DynaTest({
group("search spec") {
searchSpecTest()
}
// group("shortcuts") {
// shortcutsTestBatch()
// }
group("shortcuts") {
shortcutsTestBatch()
}


})

0 comments on commit baf3886

Please sign in to comment.