Keypress event in global search box

22 views
Skip to first unread message

Alberto Pereira

unread,
Jan 15, 2019, 6:45:59 AM1/15/19
to AtoM Users
Hi,

I've encountered a small bug in the global search box related to the keyboard events. Atom (and this is version 2.4.1) is listening for the keyboard key codes (up and down), to traverse the suggestions box list. The problem is that it's using "keyup" and "keypress" events. While "keyup" and "keydown" would be fine to get the event key codes, the "keypress" event returns the corresponding char code, not the key code. This means that, at least for me, and particularly in Chrome, there are a coincidental match between the keycode for the up keyboard key (38) and the & symbol char code (38); so, typing the & symbol not only behaves like pressing the up arrow key in the keyboard, but also isn't typed in the box. 

The problem is, apparently, limited to the global search box (I didn't really dig in to why is that), and I traced it down to js/dominion.js. 

To temporarily solve the problem, there are two ways to (easily) do this: 

1 - Just comment out the line 251 on js/dominion.js:

.on('keypress', $.proxy(this.keypress, this))

2 - Remove all keypress event listeners, by adding a piece of js code in the template:

window.addEventListener('keypress', function (event) {
   
event.stopPropagation();
}, true);


I ended up using option 2, in the _header.php file of my template, to avoid having to change js/dominion.js in the next Atom iteration; there seems to be no other consequences, by I'll be monitoring.

My question, after the long introduction: is there other, cleaner way, to do this? Can I override js/dominion,js in my template, or just remove the event listener of a specific object?

Thanks!

Alberto

Dan Gillean

unread,
Jan 16, 2019, 5:38:51 PM1/16/19
to ICA-AtoM Users
Hi Alberto, 

Thanks for bringing this to our attention AND sharing your solution! Does this solution still allow for keyboard navigation in the global search box drop-down, or is your workaround simply to disable keyboard navigation?

Funnily, I have an issue ticket that is currently waiting for my testing related this - rather, related to the fact that you can't enter the & ampersand character in the global search box because of this conflict you've identified. See: 
I am going to forward your message to the developer who implemented the fix for review, to determine which fix is a better solution, and if each solution addresses the issues outlined in the other. 

Since this issue is up for QA/Review, whatever solution we implement after some discussion and testing will be included in the 2.5 release. 

Cheers, 

Dan Gillean, MAS, MLIS
AtoM Program Manager
Artefactual Systems, Inc.
604-527-2056
@accesstomemory


--
You received this message because you are subscribed to the Google Groups "AtoM Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ica-atom-user...@googlegroups.com.
To post to this group, send email to ica-ato...@googlegroups.com.
Visit this group at https://groups.google.com/group/ica-atom-users.
To view this discussion on the web visit https://groups.google.com/d/msgid/ica-atom-users/f6b692a1-a333-486d-bd39-4b5c378dc203%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Alberto Pereira

unread,
Jan 16, 2019, 6:23:31 PM1/16/19
to ica-ato...@googlegroups.com
Hi,

sorry I didn't see there was already an issue raised.

Well, that solution is better than mine, both actually, because the first one I posted is lacking some more changes (even if cosmetic), namely changing the dominion.js "keypress" function name to keydown (just to keep things clean), and the second one is removing all event listeners for the "keypress" event globally, which can break stuff somewhere else (and I'm not absolutely sure if there's some compatibility issues with older browsers - should be tested). I just used the second because 1 - for me there's apparently no other consequences and 2 - I don't want to change Atom's core code.

Anyway, my first solution is more on track I suppose. "Keypress" event is being deprecated and should be replaced throughout all of Atom's code. The solution you presented can be implemented in the 2.5 release, and, because it's just checking for Chrome in that particular piece of code, won't break anything anywhere else and will fix the issue, but for a future release this should be addressed. Adding to this, there are other deprecations: "keyboardEvent.keyCode" or "keyboardEvent.which", for instance, should be replaced by "keyboardEvent.key" (and those are used in this file aswell).

Oh, and finally, yes, the solution I presented still maintains keyboard navigation.

Thanks for the reply.

Cheers,

Alberto

Dan Gillean

unread,
Jan 17, 2019, 4:24:34 PM1/17/19
to ICA-AtoM Users
Thank you for this useful feedback, Alberto! 

I have filed an issue ticket for us to track these deprecations and hopefully update them soon: 

Dan Gillean, MAS, MLIS
AtoM Program Manager
Artefactual Systems, Inc.
604-527-2056
@accesstomemory

Reply all
Reply to author
Forward
0 new messages