[MIRROR] Makes dropdowns better (#2986)

* [MIRROR] Makes dropdowns better (#2078) * Makes dropdowns better (#82697) ## About The Pull Request Kind of a pain to work with, confusing people with its prop names (many such cases!) After recently discovering deathmatch it's very obvious to me how broken it is, so I made it less so (now comes with a complete ui upgrade!) It now scrolls with the selection and to the selection on open, which felt like major QoL <details> <summary>pics/vids</summary> In motion ![7627sWJ2nS](https://github.com/tgstation/tgstation/assets/42397676/982427b2-6dc8-4c91-90cf-6e17d211f5ae) Deathmatch got some UI facelifts ![GAotCHxtZg](https://github.com/tgstation/tgstation/assets/42397676/769317ad-7a9f-410a-a60f-4ddfb377210c) ![Ca2UJSpxlY](https://github.com/tgstation/tgstation/assets/42397676/ea188cda-a79b-4ca0-9209-1c69f57231dc) Fixes #75741 ![image](https://github.com/tgstation/tgstation/assets/42397676/d30a1ae4-cf08-4512-9ce6-5499084647b4) </details> ## Why It's Good For The Game Better UX Bug fixes Potential exploit patched (ui validation for ai voice changer) Fixes #81506 ## Changelog :cl: fix: Dropdowns received some much-needed QoL, like having the scrollbar follow your selection. fix: AI voice changer now shows its current voice selection. fix: Deathmatch screen has been touched up. fix: Prefs menu has their dropdowns simplified, hopefully fixing issues /:cl: --------- Co-authored-by: san7890 <[email protected]> * Makes dropdowns better * Initial pass, things still broken * Initial pass, things still broken * Update LimbsPage.tsx --------- Co-authored-by: Jeremiah <[email protected]> Co-authored-by: san7890 <[email protected]> Co-authored-by: Mal <[email protected]> * Update character_voice.tsx --------- Co-authored-by: NovaBot <[email protected]> Co-authored-by: Jeremiah <[email protected]> Co-authored-by: san7890 <[email protected]> Co-authored-by: Mal <[email protected]> Co-authored-by: Iajret <[email protected]>
Fluffy-Frontier · Apr 21, 2024 · 5e7fa16 · 5e7fa16
1 parent a82655b
commit 5e7fa16
Show file tree

Hide file tree

Showing 78 changed files with 786 additions and 698 deletions.
diff --git a/code/modules/antagonists/malf_ai/malf_ai_modules.dm b/code/modules/antagonists/malf_ai/malf_ai_modules.dm
@@ -945,6 +945,8 @@ GLOBAL_LIST_INIT(malf_modules, subtypesof(/datum/ai_module))
 	var/prev_verbs
 	/// Saved span state, used to restore after a voice change
 	var/prev_span
+	/// The list of available voices
+	var/static/list/voice_options = list("normal", SPAN_ROBOT, SPAN_YELL, SPAN_CLOWN)
 
 /obj/machinery/ai_voicechanger/Initialize(mapload)
 	. = ..()
@@ -972,11 +974,12 @@ GLOBAL_LIST_INIT(malf_modules, subtypesof(/datum/ai_module))
 
 /obj/machinery/ai_voicechanger/ui_data(mob/user)
 	var/list/data = list()
-	data["voices"] = list("normal", SPAN_ROBOT, SPAN_YELL, SPAN_CLOWN) //manually adding this since i dont see other option
+	data["voices"] = voice_options
 	data["loud"] = loudvoice
 	data["on"] = changing_voice
 	data["say_verb"] = say_verb
 	data["name"] = say_name
+	data["selected"] = say_span || owner.speech_span
 	return data
 
 /obj/machinery/ai_voicechanger/ui_act(action, params)
@@ -1010,9 +1013,23 @@ GLOBAL_LIST_INIT(malf_modules, subtypesof(/datum/ai_module))
 			if(changing_voice)
 				owner.radio.use_command = loudvoice
 		if("look")
-			say_span = params["look"]
+			var/selection = params["look"]
+			if(isnull(selection))
+				return FALSE
+
+			var/found = FALSE
+			for(var/option in voice_options)
+				if(option == selection)
+					found = TRUE
+					break
+			if(!found)
+				stack_trace("User attempted to select an unavailable voice option")
+				return FALSE
+
+			say_span = selection
 			if(changing_voice)
 				owner.speech_span = say_span
+			to_chat(usr, span_notice("Voice set to [selection]."))
 		if("verb")
 			say_verb = params["verb"]
 			if(changing_voice)

diff --git a/tgui/packages/tgui/components/Dropdown.tsx b/tgui/packages/tgui/components/Dropdown.tsx
@@ -1,12 +1,12 @@
 import { classes } from 'common/react';
-import { ReactNode, useCallback, useEffect, useRef, useState } from 'react';
+import { ReactNode, useEffect, useRef, useState } from 'react';
 
 import { BoxProps, unit } from './Box';
 import { Button } from './Button';
 import { Icon } from './Icon';
 import { Popper } from './Popper';
 
-type DropdownEntry = {
+export type DropdownEntry = {
   displayText: ReactNode;
   value: string | number;
 };
@@ -19,7 +19,11 @@ type Props = {
   options: DropdownOption[];
   /** Called when a value is picked from the list, `value` is the value that was picked */
   onSelected: (value: any) => void;
+  /** Currently selected entry to display. Can be left stateless to permanently display this value. */
+  selected: DropdownOption | null | undefined;
 } & Partial<{
+  /** Whether to scroll automatically on open. Defaults to true */
+  autoScroll: boolean;
   /** Whether to display previous / next buttons */
   buttons: boolean;
   /** Whether to clip the selected text */
@@ -28,7 +32,7 @@ type Props = {
   color: string;
   /** Disables the dropdown */
   disabled: boolean;
-  /** Text to always display in place of the selected text */
+  /** Overwrites selection text with this. Good for objects etc. */
   displayText: ReactNode;
   /** Icon to display in dropdown button */
   icon: string;
@@ -44,17 +48,26 @@ type Props = {
   onClick: (event) => void;
   /** Dropdown renders over instead of below */
   over: boolean;
-  /** Currently selected entry */
-  selected: string | number;
+  /** Text to show when nothing has been selected. */
+  placeholder: string;
 }> &
   BoxProps;
 
+enum DIRECTION {
+  Previous = 'previous',
+  Next = 'next',
+  Current = 'current',
+}
+
+const NONE = -1;
+
 function getOptionValue(option: DropdownOption) {
   return typeof option === 'string' ? option : option.value;
 }
 
 export function Dropdown(props: Props) {
   const {
+    autoScroll = true,
     buttons,
     className,
     clipSelectedText = true,
@@ -70,46 +83,64 @@ export function Dropdown(props: Props) {
     onSelected,
     options = [],
     over,
+    placeholder = 'Select...',
     selected,
-    width,
+    width = '15rem',
   } = props;
 
   const [open, setOpen] = useState(false);
   const adjustedOpen = over ? !open : open;
   const innerRef = useRef<HTMLDivElement>(null);
 
-  /** Update the selected value when clicking the left/right buttons */
-  const updateSelected = useCallback(
-    (direction: 'previous' | 'next') => {
-      if (options.length < 1 || disabled) {
-        return;
-      }
-      const startIndex = 0;
-      const endIndex = options.length - 1;
+  const selectedIndex =
+    options.findIndex((option) => getOptionValue(option) === selected) || 0;
 
-      let selectedIndex = options.findIndex(
-        (option) => getOptionValue(option) === selected,
-      );
+  function scrollTo(position: number) {
+    let scrollPos = position;
+    if (position < selectedIndex) {
+      scrollPos = position < 2 ? 0 : position - 2;
+    } else {
+      scrollPos =
+        position > options.length - 3 ? options.length - 1 : position - 2;
+    }
 
-      if (selectedIndex < 0) {
-        selectedIndex = direction === 'next' ? endIndex : startIndex;
-      }
+    const element = innerRef.current?.children[scrollPos];
+    element?.scrollIntoView({ block: 'nearest' });
+  }
 
-      let newIndex = selectedIndex;
-      if (direction === 'next') {
-        newIndex = selectedIndex === endIndex ? startIndex : ++selectedIndex;
-      } else {
-        newIndex = selectedIndex === startIndex ? endIndex : --selectedIndex;
-      }
-
-      onSelected?.(getOptionValue(options[newIndex]));
-    },
-    [disabled, onSelected, options, selected],
-  );
+  /** Update the selected value when clicking the left/right buttons */
+  function updateSelected(direction: DIRECTION) {
+    if (options.length < 1 || disabled) {
+      return;
+    }
+
+    const startIndex = 0;
+    const endIndex = options.length - 1;
+
+    let newIndex: number;
+    if (selectedIndex < 0) {
+      newIndex = direction === 'next' ? endIndex : startIndex; // No selection yet
+    } else if (direction === 'next') {
+      newIndex = selectedIndex === endIndex ? startIndex : selectedIndex + 1; // Move to next option
+    } else {
+      newIndex = selectedIndex === startIndex ? endIndex : selectedIndex - 1; // Move to previous option
+    }
+
+    if (open && autoScroll) {
+      scrollTo(newIndex);
+    }
+    onSelected?.(getOptionValue(options[newIndex]));
+  }
 
   /** Allows the menu to be scrollable on open */
   useEffect(() => {
-    if (!open) return;
+    if (!open) {
+      return;
+    }
+
+    if (autoScroll && selectedIndex !== NONE) {
+      scrollTo(selectedIndex);
+    }
 
     innerRef.current?.focus();
   }, [open]);
@@ -151,69 +182,64 @@ export function Dropdown(props: Props) {
         </div>
       }
     >
-      <div>
-        <div className="Dropdown" style={{ width: unit(width) }}>
-          <div
-            className={classes([
-              'Dropdown__control',
-              'Button',
-              'Button--dropdown',
-              'Button--color--' + color,
-              disabled && 'Button--disabled',
-              className,
-            ])}
-            onClick={(event) => {
-              if (disabled && !open) {
-                return;
-              }
-              setOpen(!open);
-              onClick?.(event);
+      <div className="Dropdown" style={{ width: unit(width) }}>
+        <div
+          className={classes([
+            'Dropdown__control',
+            'Button',
+            'Button--dropdown',
+            'Button--color--' + color,
+            disabled && 'Button--disabled',
+            className,
+          ])}
+          onClick={(event) => {
+            if (disabled && !open) {
+              return;
+            }
+            setOpen(!open);
+            onClick?.(event);
+          }}
+        >
+          {icon && (
+            <Icon mr={1} name={icon} rotation={iconRotation} spin={iconSpin} />
+          )}
+          <span
+            className="Dropdown__selected-text"
+            style={{
+              overflow: clipSelectedText ? 'hidden' : 'visible',
             }}
           >
-            {icon && (
-              <Icon
-                mr={1}
-                name={icon}
-                rotation={iconRotation}
-                spin={iconSpin}
-              />
-            )}
-            <span
-              className="Dropdown__selected-text"
-              style={{
-                overflow: clipSelectedText ? 'hidden' : 'visible',
-              }}
-            >
-              {displayText || selected}
+            {displayText ||
+              (selected && getOptionValue(selected)) ||
+              placeholder}
+          </span>
+          {!noChevron && (
+            <span className="Dropdown__arrow-button">
+              <Icon name={adjustedOpen ? 'chevron-up' : 'chevron-down'} />
             </span>
-            {!noChevron && (
-              <span className="Dropdown__arrow-button">
-                <Icon name={adjustedOpen ? 'chevron-up' : 'chevron-down'} />
-              </span>
-            )}
-          </div>
-          {buttons && (
-            <>
-              <Button
-                disabled={disabled}
-                height={1.8}
-                icon="chevron-left"
-                onClick={() => {
-                  updateSelected('previous');
-                }}
-              />
-
-              <Button
-                disabled={disabled}
-                height={1.8}
-                icon="chevron-right"
-                onClick={() => {
-                  updateSelected('next');
-                }}
-              />
-            </>
           )}
         </div>
+        {buttons && (
+          <>
+            <Button
+              disabled={disabled}
+              height={1.8}
+              icon="chevron-left"
+              onClick={() => {
+                updateSelected(DIRECTION.Previous);
+              }}
+            />
+
+            <Button
+              disabled={disabled}
+              height={1.8}
+              icon="chevron-right"
+              onClick={() => {
+                updateSelected(DIRECTION.Next);
+              }}
+            />
+          </>
+        )}
       </div>
     </Popper>
   );

diff --git a/tgui/packages/tgui/interfaces/AdminFax.jsx b/tgui/packages/tgui/interfaces/AdminFax.jsx
@@ -63,10 +63,9 @@ export const FaxMainPanel = (props) => {
         <Box fontSize="13px">
           <Dropdown
             textAlign="center"
-            selected="Choose fax machine..."
+            placeholder="Choose fax machine..."
             width="100%"
-            noChevron
-            nowrap
+            selected={fax}
             options={data.faxes}
             onSelected={(value) => setFax(value)}
           />

diff --git a/tgui/packages/tgui/interfaces/AdminPDA.jsx b/tgui/packages/tgui/interfaces/AdminPDA.jsx
@@ -33,7 +33,8 @@ const ReceiverChoice = (props) => {
         <Dropdown
           disabled={spam}
           selected={user}
-          displayText={user ? users[user].username : 'Pick a user...'}
+          displayText={users[user]?.username}
+          placeholder="Pick a user..."
           options={receivers
             .filter((rcvr) => showInvisible || !rcvr.invisible)
             .map((rcvr) => ({