Skip to content

Commit

Permalink
Fixes Using 'ESC' Key To Unbind Keybindings + Adds Function to Check …
Browse files Browse the repository at this point in the history
…It Elsewhere (tgstation#83710)

## About The Pull Request

Consequence of tgstation#80335 (d3554b3)


![image](https://github.com/tgstation/tgstation/assets/34697715/cce22c30-e4d0-4f92-b8af-de908fbcce28)

Likely some weird IE shit, we get "Esc" instead of "Escape".

I added it as a function so this is a bit hardier to potentially
breaking so it doesn't go haywire silently again when we migrate to
Webview2. It's also likely that a weird combo of OS and IE and BYOND
Version and whatever will spit out "Esc" instead of "Escape", so let's
just have a helper function to access both so we don't have to worry
about it any more.

✅: Works on my machine
## Why It's Good For The Game

Players should be allowed to unbind whatever keys they want, regressions
are bad. New helper function that can check both cases (since we're
unsure of why either case pops up and it's unlikely that we have control
over it) was implemented in every spot where we were checking
`KEY.Escape` in this PR as well.

## Changelog
:cl:
fix: Using the 'ESC' key on your keyboard to unbind a key in the
keybindings preferences menu should now work as expected. This should
also be fixed for people in a variety of other spots too.
/:cl:

My javascript is a bit bad let me know if I'm doing something wack
  • Loading branch information
san7890 authored Jun 6, 2024
1 parent 486ebf6 commit f7ad080
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 30 deletions.
19 changes: 19 additions & 0 deletions tgui/packages/common/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Handles modifier keys (Shift, Alt, Control) and arrow keys.
*
* For alphabetical keys, use the actual character (e.g. 'a') instead of the key code.
* Don't access Esc or Escape directly, use isEscape() instead
*
* Something isn't here that you want? Just add it:
* @url https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
Expand All @@ -16,6 +17,8 @@
* // do something
* }
* ```
*
*
*/
export enum KEY {
Alt = 'Alt',
Expand All @@ -25,6 +28,7 @@ export enum KEY {
Down = 'ArrowDown',
End = 'End',
Enter = 'Enter',
Esc = 'Esc',
Escape = 'Escape',
Home = 'Home',
Insert = 'Insert',
Expand All @@ -37,3 +41,18 @@ export enum KEY {
Tab = 'Tab',
Up = 'ArrowUp',
}

/**
* ### isEscape
*
* Checks if the user has hit the 'ESC' key on their keyboard.
* There's a weirdness in BYOND where this could be either the string
* 'Escape' or 'Esc' depending on the browser. This function handles
* both cases.
*
* @param key - the key to check, typically from event.key
* @returns true if key is Escape or Esc, false otherwise
*/
export function isEscape(key: string): boolean {
return key === KEY.Esc || key === KEY.Escape;
}
9 changes: 5 additions & 4 deletions tgui/packages/tgui-say/TguiSay.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { BooleanLike } from 'common/react';
import { Component, createRef, RefObject } from 'react';
import { dragStartHandler } from 'tgui/drag';
Expand Down Expand Up @@ -245,9 +245,10 @@ export class TguiSay extends Component<{}, State> {
this.handleIncrementChannel();
break;

case KEY.Escape:
this.handleClose();
break;
default:
if (isEscape(event.key)) {
this.handleClose();
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions tgui/packages/tgui/components/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { Placement } from '@popperjs/core';
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { BooleanLike, classes } from 'common/react';
import {
ChangeEvent,
Expand Down Expand Up @@ -131,7 +131,7 @@ export const Button = (props: Props) => {
}

// Refocus layout on pressing escape.
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
event.preventDefault();
}
}}
Expand Down Expand Up @@ -343,7 +343,7 @@ const ButtonInput = (props: InputProps) => {
commitResult(event);
return;
}
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
setInInput(false);
}
}}
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/components/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* @license MIT
*/

import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { classes } from 'common/react';
import { debounce } from 'common/timer';
import { KeyboardEvent, SyntheticEvent, useEffect, useRef } from 'react';
Expand Down Expand Up @@ -127,7 +127,7 @@ export function Input(props: Props) {
return;
}

if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
onEscape?.(event);

event.currentTarget.value = toInputValue(value);
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/components/NumberInput.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { clamp } from 'common/math';
import { BooleanLike, classes } from 'common/react';
import {
Expand Down Expand Up @@ -239,7 +239,7 @@ export class NumberInput extends Component<Props, State> {
onChange?.(targetValue);
onDrag?.(targetValue);
}
} else if (event.key === KEY.Escape) {
} else if (isEscape(event.key)) {
this.setState({
editing: false,
});
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/components/TextArea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @license MIT
*/

import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { classes } from 'common/react';
import {
forwardRef,
Expand Down Expand Up @@ -82,7 +82,7 @@ export const TextArea = forwardRef(
return;
}

if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
onEscape?.(event);
if (selfClear) {
event.currentTarget.value = '';
Expand Down
11 changes: 7 additions & 4 deletions tgui/packages/tgui/interfaces/AlertModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { BooleanLike } from 'common/react';
import { KeyboardEvent, useState } from 'react';

Expand Down Expand Up @@ -55,9 +55,6 @@ export function AlertModal(props) {
case KEY.Enter:
act('choose', { choice: buttons[selected] });
return;
case KEY.Escape:
act('cancel');
return;
case KEY.Left:
event.preventDefault();
onKey(DIRECTION.Decrement);
Expand All @@ -67,6 +64,12 @@ export function AlertModal(props) {
event.preventDefault();
onKey(DIRECTION.Increment);
return;

default:
if (isEscape(event.key)) {
act('cancel');
return;
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions tgui/packages/tgui/interfaces/KeyComboModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { useState } from 'react';

import { useBackend, useLocalState } from '../backend';
Expand All @@ -20,7 +20,7 @@ const isStandardKey = (event: React.KeyboardEvent<HTMLDivElement>): boolean => {
event.key !== KEY.Alt &&
event.key !== KEY.Control &&
event.key !== KEY.Shift &&
event.key !== KEY.Escape
!isEscape(event.key)
);
};

Expand Down Expand Up @@ -97,7 +97,7 @@ export const KeyComboModal = (props) => {
if (event.key === KEY.Enter) {
act('submit', { entry: input });
}
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
act('cancel');
}
return;
Expand All @@ -109,7 +109,7 @@ export const KeyComboModal = (props) => {
setValue(formatKeyboardEvent(event));
setBinding(false);
return;
} else if (event.key === KEY.Escape) {
} else if (isEscape(event.key)) {
setValue(init_value);
setBinding(false);
return;
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/interfaces/LootPanel/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape } from 'common/keys';
import { BooleanLike } from 'common/react';
import { useState } from 'react';

Expand Down Expand Up @@ -27,7 +27,7 @@ export function LootPanel(props) {
<Window height={275} width={190} title={`Contents: ${total}`}>
<Window.Content
onKeyDown={(event) => {
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
Byond.sendMessage('close');
}
}}
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/interfaces/NumberInputModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { useState } from 'react';

import { useBackend } from '../backend';
Expand Down Expand Up @@ -44,7 +44,7 @@ export const NumberInputModal = (props) => {
if (event.key === KEY.Enter) {
act('submit', { entry: input });
}
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
act('cancel');
}
}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { range, sortBy } from 'common/collections';
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { Component } from 'react';

import { resolveAsset } from '../../assets';
Expand Down Expand Up @@ -42,7 +42,7 @@ const isStandardKey = (event: KeyboardEvent): boolean => {
event.key !== KEY.Alt &&
event.key !== KEY.Control &&
event.key !== KEY.Shift &&
event.key !== KEY.Escape
!isEscape(event.key)
);
};

Expand Down Expand Up @@ -287,7 +287,7 @@ export class KeybindingsPage extends Component<{}, KeybindingsPageState> {
if (isStandardKey(event)) {
this.setRebindingHotkey(formatKeyboardEvent(event));
return;
} else if (event.key === KEY.Escape) {
} else if (isEscape(event.key)) {
this.setRebindingHotkey(undefined);
return;
}
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/interfaces/TextInputModal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEY } from 'common/keys';
import { isEscape, KEY } from 'common/keys';
import { KeyboardEvent, useState } from 'react';

import { useBackend } from '../backend';
Expand Down Expand Up @@ -67,7 +67,7 @@ export const TextInputModal = (props) => {
) {
act('submit', { entry: input });
}
if (event.key === KEY.Escape) {
if (isEscape(event.key)) {
act('cancel');
}
}}
Expand Down

0 comments on commit f7ad080

Please sign in to comment.