From 405cdfdc11b69cb5a5ca1b1c187d9a08250a89b2 Mon Sep 17 00:00:00 2001 From: Kyung Min Shin Date: Wed, 2 Dec 2020 23:54:14 -0500 Subject: [PATCH] Refactor classifications form to use rjsf (#1454) --- skyportal/tests/frontend/test_sources.py | 11 +- static/js/components/ClassificationForm.css | 6 - static/js/components/ClassificationForm.jsx | 427 +++++++++++--------- static/js/components/SourceDesktop.jsx | 1 + static/js/components/SourceMobile.jsx | 2 +- 5 files changed, 240 insertions(+), 207 deletions(-) delete mode 100644 static/js/components/ClassificationForm.css diff --git a/skyportal/tests/frontend/test_sources.py b/skyportal/tests/frontend/test_sources.py index b1bc4548e5..8e6cd840e0 100644 --- a/skyportal/tests/frontend/test_sources.py +++ b/skyportal/tests/frontend/test_sources.py @@ -93,17 +93,20 @@ def test_classifications(driver, user, taxonomy_token, public_group, public_sour driver.get(f"/become_user/{user.id}") driver.get(f"/source/{public_source.id}") driver.wait_for_xpath(f'//div[text()="{public_source.id}"]') - driver.click_xpath('//div[@id="tax-select"]') + driver.click_xpath('//div[@id="root_taxonomy"]') driver.click_xpath( f'//*[text()="{tax_name} ({tax_version})"]', wait_clickable=False, scroll_parent=True, ) driver.click_xpath('//*[@id="classification"]') - driver.wait_for_xpath('//*[@id="classification"]').send_keys( - "Symmetrical", Keys.ENTER + driver.click_xpath('//li[contains(@data-value, "Symmetrical")]', scroll_parent=True) + driver.click_xpath('//*[@id="probability"]') + driver.wait_for_xpath('//*[@id="probability"]').send_keys("1", Keys.ENTER) + driver.click_xpath( + "//*[@id='classifications-content']//span[text()='Submit']", + wait_clickable=False, ) - driver.click_xpath("//*[@id='classificationSubmitButton']") # Notification driver.wait_for_xpath("//*[text()='Classification saved']") diff --git a/static/js/components/ClassificationForm.css b/static/js/components/ClassificationForm.css deleted file mode 100644 index 994d582bd2..0000000000 --- a/static/js/components/ClassificationForm.css +++ /dev/null @@ -1,6 +0,0 @@ -.classificationEntry { - position: relative; -} -.inputDiv { - padding: 0.3rem; -} diff --git a/static/js/components/ClassificationForm.jsx b/static/js/components/ClassificationForm.jsx index f1bfaa9631..b58129e730 100644 --- a/static/js/components/ClassificationForm.jsx +++ b/static/js/components/ClassificationForm.jsx @@ -1,52 +1,55 @@ -import React, { useReducer } from "react"; -import { useDispatch } from "react-redux"; +import React from "react"; +import { useDispatch, useSelector } from "react-redux"; import PropTypes from "prop-types"; -import Select from "@material-ui/core/Select"; -import InputLabel from "@material-ui/core/InputLabel"; import TextField from "@material-ui/core/TextField"; -import Button from "@material-ui/core/Button"; import MenuItem from "@material-ui/core/MenuItem"; -import Autocomplete from "@material-ui/lab/Autocomplete"; +import Select from "@material-ui/core/Select"; +import Input from "@material-ui/core/Input"; +import InputLabel from "@material-ui/core/InputLabel"; +import Chip from "@material-ui/core/Chip"; import { makeStyles } from "@material-ui/core/styles"; +import Form from "@rjsf/material-ui"; + import { showNotification } from "baselayer/components/Notifications"; import * as Actions from "../ducks/source"; const useStyles = makeStyles(() => ({ - taxonomySelect: { - maxWidth: "100%", - fullWidth: "true", + chips: { display: "flex", - wrap: "nowrap", + flexWrap: "wrap", + }, + chip: { + margin: 2, }, })); // For each node in the hierarchy tree, add its full path from root -// to the node_paths list -const add_node_paths = (node_paths, hierarchy, prefix_path = []) => { - const this_node_path = [...prefix_path]; +// to the nodePaths list +const addNodePaths = (nodePaths, hierarchy, prefix_path = []) => { + const thisNodePath = [...prefix_path]; if ( hierarchy.class !== undefined && hierarchy.class !== "Time-domain Source" ) { - this_node_path.push(hierarchy.class); - node_paths.push(this_node_path); + thisNodePath.push(hierarchy.class); + nodePaths.push(thisNodePath); } hierarchy.subclasses?.forEach((item) => { if (typeof item === "object") { - add_node_paths(node_paths, item, this_node_path); + addNodePaths(nodePaths, item, thisNodePath); } }); }; // For each class in the hierarchy, return its name // as well as the path from the root of hierarchy to that class -const allowed_classes = (hierarchy) => { - const class_paths = []; - add_node_paths(class_paths, hierarchy); +const allowedClasses = (hierarchy) => { + const classPaths = []; + addNodePaths(classPaths, hierarchy); - const classes = class_paths.map((path) => ({ + const classes = classPaths.map((path) => ({ class: path.pop(), context: path.reverse(), })); @@ -56,197 +59,229 @@ const allowed_classes = (hierarchy) => { const ClassificationForm = ({ obj_id, taxonomyList }) => { const classes = useStyles(); + const dispatch = useDispatch(); + const groups = useSelector((state) => state.groups.userAccessible); + const groupIDToName = {}; + groups.forEach((g) => { + groupIDToName[g.id] = g.name; + }); + const ITEM_HEIGHT = 48; + const MenuProps = { + PaperProps: { + style: { + maxHeight: ITEM_HEIGHT * 4.5, + width: 250, + }, + }, + }; + const latestTaxonomyList = taxonomyList.filter((t) => t.isLatest); - function reducer(state, action) { - switch (action.name) { - case "taxonomy_index": - return { - ...state, - [action.name]: action.value, - allowed_classes: allowed_classes( - latestTaxonomyList[action.value].hierarchy - ), - }; - default: - return { - ...state, - [action.name]: action.value, - }; + const handleSubmit = async ({ formData }) => { + // Get the classification without the context + const classification = formData.classification.split(" <> ")[0]; + const data = { + taxonomy_id: parseInt(formData.taxonomy, 10), + obj_id, + classification, + probability: formData.probability, + }; + if (formData.groupIDs) { + data.group_ids = formData.groupIDs.map((id) => parseInt(id, 10)); + } + const result = await dispatch(Actions.addClassification(data)); + if (result.status === "success") { + dispatch(showNotification("Classification saved")); } - } - - const reduxDispatch = useDispatch(); - - const initialState = { - isSubmitting: false, - taxonomy_index: latestTaxonomyList.length > 0 ? 0 : null, - classification: null, - probability: 1.0, - class_select_enabled: false, - probability_select_enabled: false, - probability_errored: false, - allowed_classes: - latestTaxonomyList.length > 0 - ? latestTaxonomyList[0].allowed_classes - : [null], }; - const [state, localDispatch] = useReducer(reducer, initialState); - if (latestTaxonomyList.length === 0) { - return No taxonomies loaded...; - } + // Custom form widget for the classifications to format and display the contexts as well + const CustomClassificationWidget = ({ value, onChange, options }) => { + return ( + { + onChange(event.target.value); + }} + > + {options.enumOptions.map((option) => { + const [classification, context] = option.value.split(" <> "); + return ( + + + {classification} +   + {context !== "" &&
} + {context} + + + ); + })} + + ); + }; + CustomClassificationWidget.propTypes = { + value: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + options: PropTypes.shape({ + enumOptions: PropTypes.arrayOf(PropTypes.shape({})), + }).isRequired, + }; - const handleTaxonomyChange = (event) => { - localDispatch({ name: "taxonomy_index", value: event.target.value }); - localDispatch({ name: "classification", value: "" }); - localDispatch({ name: "class_select_enabled", value: true }); - localDispatch({ name: "probability_select_enabled", value: false }); - localDispatch({ name: "probability_errored", value: false }); - localDispatch({ name: "probability", value: 1.0 }); + // Custom form widget for probability because rxjs MUI UpdownWidget does not have working min/max/step + // https://github.com/rjsf-team/react-jsonschema-form/issues/2022 + const CustomProbabilityWidget = ({ value, onChange }) => { + return ( + { + onChange(event.target.value); + }} + /> + ); + }; + CustomProbabilityWidget.propTypes = { + value: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, }; - const handleClasschange = (event, value) => { - localDispatch({ name: "classification", value }); - localDispatch({ name: "probability_select_enabled", value: true }); - localDispatch({ name: "probability", value: 1.0 }); + const CustomGroupsWidget = ({ value, onChange, options }) => { + return ( + <> + + Choose Group (all user-accessible groups if blank) + + } + labelId="classificationGroupSelectLabel" + value={value || ""} + renderValue={(selected) => ( +
+ {selected.map((group) => ( + + ))} +
+ )} + MenuProps={MenuProps} + fullWidth + multiple + > + {options.enumOptions.length > 0 && + options.enumOptions.map((group) => ( + + {group.label} + + ))} + + + ); + }; + CustomGroupsWidget.propTypes = { + value: PropTypes.string.isRequired, + onChange: PropTypes.func.isRequired, + options: PropTypes.shape({ + enumOptions: PropTypes.arrayOf(PropTypes.shape({})), + }).isRequired, }; - const processProb = (event) => { - // make sure that the probability in in [0,1], otherwise set - // an error state on the entry - if ( - Number.isNaN(parseFloat(event.target.value)) || - parseFloat(event.target.value) > 1 || - parseFloat(event.target.value) < 0 - ) { - localDispatch({ name: "probability_errored", value: true }); - } else { - localDispatch({ name: "probability_errored", value: false }); - localDispatch({ name: "probability", value: event.target.value }); - } + const widgets = { + customClassificationWidget: CustomClassificationWidget, + customProbabilityWidget: CustomProbabilityWidget, + customGroupsWidget: CustomGroupsWidget, }; - const onSubmit = async (event) => { - // TODO: allow fine-grained user groups in submission - event.preventDefault(); - localDispatch({ name: "isSubmitting", value: true }); - const formData = { - taxonomy_id: latestTaxonomyList[state.taxonomy_index].id, - obj_id, - classification: state.classification.class, - probability: parseFloat(state.probability), - }; - const result = await reduxDispatch(Actions.addClassification(formData)); - if (result.status === "success") { - reduxDispatch(showNotification("Classification saved")); - } - localDispatch({ name: "isSubmitting", value: false }); + const formSchema = { + description: "Add Classification", + type: "object", + required: ["taxonomy", "classification", "probability"], + properties: { + groupIDs: { + type: "array", + items: { + type: "string", + enum: groups.map((group) => group.id.toString()), + enumNames: groups.map((group) => group.name), + }, + uniqueItems: true, + }, + taxonomy: { + type: "string", + title: "Taxonomy", + enum: latestTaxonomyList.map((taxonomy) => taxonomy.id.toString()), + enumNames: latestTaxonomyList.map( + (taxonomy) => `${taxonomy.name} (${taxonomy.version})` + ), + }, + }, + dependencies: { + taxonomy: { + oneOf: [], + }, + }, + }; + latestTaxonomyList.forEach((taxonomy) => { + const currentClasses = allowedClasses(taxonomy.hierarchy).map( + (option) => + `${option.class} <> ${ + option.context.length > 0 ? option.context.join(" « ") : "" + }` + ); + formSchema.dependencies.taxonomy.oneOf.push({ + properties: { + taxonomy: { + enum: [taxonomy.id.toString()], + }, + classification: { + type: "string", + name: "classification", + title: "Classification", + enum: currentClasses, + }, + probability: { + type: "number", + name: "probability", + title: "Probability", + }, + }, + }); + }); + const uiSchema = { + groupIDs: { "ui:widget": "customGroupsWidget" }, + classification: { "ui:widget": "customClassificationWidget" }, + probability: { "ui:widget": "customProbabilityWidget" }, }; return ( -
-
-
-

Add Classification

- Taxonomy - -
- { - if ( - state.classification === null || - state.classification === "" - ) { - return true; - } - if (state.classification.class === "") { - return true; - } - return state.classification.class === option.class; - }} - value={state.classification || ""} - onChange={handleClasschange} - getOptionLabel={(option) => option.class || ""} - renderInput={(params) => ( - - )} - renderOption={ - // Note: these come from "options", which in turn come from state.allowed_classes - // See the allowed_classes function defined above - (option) => ( - - {option.class} -   - {option.context.length > 0 &&
} - {option.context.join(" « ")} - - ) - } - /> -
-
- -
-
- -
-
-
+
); }; diff --git a/static/js/components/SourceDesktop.jsx b/static/js/components/SourceDesktop.jsx index 4750d70d03..40a71540b0 100644 --- a/static/js/components/SourceDesktop.jsx +++ b/static/js/components/SourceDesktop.jsx @@ -95,6 +95,7 @@ export const useSourceStyles = makeStyles((theme) => ({ classifications: { display: "flex", flexDirection: "column", + width: "100%", }, alignRight: { display: "inline-block", diff --git a/static/js/components/SourceMobile.jsx b/static/js/components/SourceMobile.jsx index 036725a255..8f978e62c7 100644 --- a/static/js/components/SourceMobile.jsx +++ b/static/js/components/SourceMobile.jsx @@ -102,7 +102,7 @@ export const useSourceStyles = makeStyles((theme) => ({ display: "flex", flexDirection: "column", margin: "auto", - maxWidth: "100%", + width: "100%", }, thumbnails: { "& > div": {