Fix rating selection and alignment

Author: sezanzeb

frontend/src/components/pages/rating-form.tsx

@@ -5,14 +5,15 @@ import {
   RadioGroup,
   FormControlLabel,
   Radio,
-  Alert,
   Typography,
   Tooltip,
 } from "@mui/material";
 import { api } from "../../hooks/use-api";
 import { useLoginContext } from "../../contexts/login-context";
 import { Button } from "../base/button";
 import { CriterionDTO, ProjectDTO, RatingDTO } from "../../api/types/dto";
+import { useNotificationContext } from "../../contexts/notification-context";
+
 interface IRatingFormProps {
   rating?: RatingDTO;
   criterion: CriterionDTO;
@@ -31,30 +32,30 @@ export const RatingForm = ({
   const loginState = useLoginContext();
   const { user } = loginState;
 
-  const [ratingValue, setRatingValue] = useState<string | undefined>(
-    rating?.rating?.toString(),
-  );
+  const { showNotification } = useNotificationContext();
+
+  const [ratingValue, setRatingValue] = useState<number>(rating?.rating || 3);
   const [isSubmitting, setIsSubmitting] = useState(false);
-  const [error, setError] = useState<string | null>(null);
 
   React.useEffect(() => {
     if (rating) {
-      setRatingValue(rating.rating.toString());
+      setRatingValue(rating.rating);
     }
   }, [rating]);
 
   const handleSubmit = async () => {
     setIsSubmitting(true);
-    setError(null);
 
     await api.createRating({
       criterion,
-      rating: parseInt(ratingValue ?? "0", 10),
+      rating: ratingValue,
       user: user!,
       project,
     } as unknown as RatingDTO);
 
     setIsSubmitting(false);
+
+    showNotification(`Submitted ${ratingValue} for "${criterion.title}"`);
   };
 
   return (
@@ -73,38 +74,39 @@ export const RatingForm = ({
         justifyContent="center"
       >
         <Tooltip title={criterion.description}>
-          <Typography variant="h6" sx={{ cursor: "help" }}>
+          <Typography
+            style={{ flex: 1, textAlign: "right" }}
+            sx={{ cursor: "help" }}
+          >
             {criterion.title}
           </Typography>
         </Tooltip>
-
         <FormControl component="fieldset">
           <RadioGroup
             row
-            value={ratingValue}
-            onChange={(e) => setRatingValue(e.target.value)}
+            value={ratingValue?.toString()}
+            onChange={(e) => setRatingValue(parseInt(e.target.value, 10))}
           >
             {[1, 2, 3, 4, 5].map((value) => (
               <FormControlLabel
-                key={value}
+                key={value.toString()}
                 value={value.toString()}
                 control={<Radio disabled={isSubmitting} />}
                 label={value.toString()}
               />
             ))}
           </RadioGroup>
         </FormControl>
-
-        <Button
-          onClick={handleSubmit}
-          disable={isSubmitting}
-          loading={isSubmitting}
-        >
-          Submit
-        </Button>
+        <div style={{ flex: 1 }}>
+          <Button
+            onClick={handleSubmit}
+            disable={isSubmitting}
+            loading={isSubmitting}
+          >
+            Submit
+          </Button>
+        </div>
       </Stack>
-
-      {error && <Alert severity="error">{error}</Alert>}
     </div>
   );
 };

frontend/src/components/settings/project-rating/rating-criteria-settings.tsx

@@ -4,21 +4,15 @@ import type { CriterionDTO, SettingsDTO } from "../../../api/types/dto";
 import { Spacer } from "../../base/flex";
 import { SettingsSection } from "../settings-section";
 import { Subsubheading } from "../../base/headings";
-import {
-  TextField,
-  Switch,
-  FormControlLabel,
-  Stack,
-  Button,
-} from "@mui/material";
+import { Button } from "../../base/button";
+import { TextField, Switch, FormControlLabel, Stack } from "@mui/material";
 import { api } from "../../../hooks/use-api";
+import { useNotificationContext } from "../../../contexts/notification-context";
 
 // TODO Seems more maintainable to me if the save button is fine-grained
 //  and not global. Remove the global Save button and add one individually for
 //  each SettingsSection component for consistency.
 
-// TODO use our own button style
-
 interface ICriterionEditorProps {
   criterion: CriterionDTO;
   onSave: (criterion: CriterionDTO) => Promise<void>;
@@ -34,31 +28,41 @@ const CriterionEditor = React.memo(
     const [title, setTitle] = useState(criterion.title);
     const [description, setDescription] = useState(criterion.description);
 
+    const validateAndSave = (changedCriterion: CriterionDTO) => {
+      if (changedCriterion.title.length === 0) {
+        return;
+      }
+
+      if (changedCriterion.description.length === 0) {
+        return;
+      }
+
+      onSave(changedCriterion);
+    };
+
     return (
       <Stack direction={{ sm: "column", md: "row" }} spacing={{ xs: 1, sm: 2 }}>
         <TextField
+          error={title.length === 0}
           value={title}
           onChange={(event) => setTitle(event.target.value)}
-          placeholder="title"
+          placeholder="Title"
           rows={1}
         />
         <TextField
+          error={description.length === 0}
           value={description}
           onChange={(event) => setDescription(event.target.value)}
           placeholder="Description"
           rows={1}
           sx={{ flex: 1 }}
         />
         <Button
-          fullWidth={false}
-          variant="contained"
-          onClick={() => onSave({ ...criterion, title, description })}
+          onClick={() => validateAndSave({ ...criterion, title, description })}
         >
           Save
         </Button>
-        <Button variant="contained" onClick={() => onDelete(criterion)}>
-          Delete
-        </Button>
+        <Button onClick={() => onDelete(criterion)}>Delete</Button>
       </Stack>
     );
   },
@@ -72,6 +76,8 @@ export const ProjectRatingSettings = () => {
   const [allCriteria, setAllCriteria] = useState<CriterionDTO[]>([]);
   const [settings, setSettings] = useState<Partial<SettingsDTO>>({});
 
+  const { showNotification } = useNotificationContext();
+
   // Do this only on mount
   useEffect(() => {
     api.getAllCriteria().then((criteria) => {
@@ -92,8 +98,8 @@ export const ProjectRatingSettings = () => {
 
   const addCriterion = useCallback(async (): Promise<void> => {
     const newCriterion = await api.createCriterion({
-      title: "title",
-      description: "description",
+      title: "",
+      description: "",
     } as unknown as CriterionDTO);
     setAllCriteria((prev) => [...prev, newCriterion]);
   }, []);
@@ -108,6 +114,8 @@ export const ProjectRatingSettings = () => {
             : criterion;
         }),
       );
+
+      showNotification("Saved criterion");
     },
     [],
   );
@@ -168,9 +176,7 @@ export const ProjectRatingSettings = () => {
         ))}
       </div>
       <div>
-        <Button fullWidth={false} variant="contained" onClick={addCriterion}>
-          Add
-        </Button>
+        <Button onClick={addCriterion}>Add</Button>
         <Spacer />
       </div>
     </SettingsSection>