sipb / hydrant

MIT semester course planning app
https://hydrant.mit.edu/
MIT License
27 stars 18 forks source link

Add form to color picker #32

Closed fprasx closed 1 year ago

fprasx commented 1 year ago

Also update button behaviour so that only manual section assignment or color assignment is toggled at once. Renaming and color assignment are deliberately left simultaneous in the case of a custom activity.

cjquines commented 1 year ago

hm, we shd really get a formatter post commit or something

psvenk commented 1 year ago

Also, ColorButton is now dead code.

fprasx commented 1 year ago

Concerns addressed. Shrinking the buttons is tough as they automatically fill the width of the container but making them shrink to their text content causes them to all have different sizes. Therefore, I shrunk the input form so the buttons expand to be smaller. Screenshot 2023-03-05 at 1 31 16 PM

psvenk commented 1 year ago

Thanks for the changes. One more thing: what do you think of doing something like the following to make unfocus equivalent to submit?

diff --git a/src/components/ActivityButtons.tsx b/src/components/ActivityButtons.tsx
index 0a853b7..eb1e198 100644
--- a/src/components/ActivityButtons.tsx
+++ b/src/components/ActivityButtons.tsx
@@ -121,19 +121,22 @@ function ActivityColor(props: {

   const isError = input !== "" ? canonicalizeColor(input) === undefined : false;

+  const handleSubmit = (e: FormEvent) => {
+    e.preventDefault();
+    const canon = canonicalizeColor(input);
+    if (canon) {
+      setColor(canon);
+      setInput("");
+    }
+  }
+
   return (
     <Flex gap={2}>
       <HexColorPicker color={color} onChange={setColor} />
       <Flex direction="column" gap={2}>
         <form
-          onSubmit={(e) => {
-            e.preventDefault();
-            const canon = canonicalizeColor(input);
-            if (canon) {
-              setColor(canon);
-              setInput("");
-            }
-          }}
+          onSubmit={handleSubmit}
+          onBlur={handleSubmit}
         >
           <Input
             // Wide enough to hold everything, but keeps buttons below small
fprasx commented 1 year ago

Seems good.

cjquines commented 1 year ago

@psvenk you know you can like. merge this