Skip to content

Commit

Permalink
refactor(bc): minor adjustements
Browse files Browse the repository at this point in the history
  • Loading branch information
hdinia committed Mar 28, 2024
1 parent bae14df commit 09f2865
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 66 deletions.
4 changes: 0 additions & 4 deletions webapp/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,6 @@ export interface LinkClusterElement {
name: string;
}

// TODO wrong types
// LinkWithClusters
// ClusterWithLinks
// Combine both
export interface LinkClusterItem {
element: LinkClusterElement;
item_list: LinkClusterElement[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,14 @@ interface Props {

const defaultValues = {
name: "",
group: "",
group: "default",
enabled: true,
timeStep: TimeStep.HOURLY,
operator: BindingConstraintOperator.LESS,
comments: "",
} as const;
};

// TODO rename AddConstraintDialog
// TODO refactor and add group field
function AddDialog({
open,
onClose,
Expand Down Expand Up @@ -74,7 +73,7 @@ function AddDialog({
// Event Handlers
////////////////////////////////////////////////////////////////

const handleSubmit = async ({
const handleSubmit = ({
values,
}: SubmitHandlerPlus<typeof defaultValues>) => {
return createBindingConstraint(study.id, values);
Expand All @@ -88,17 +87,17 @@ function AddDialog({
* !WARNING: Current Implementation Issues & Future Directions
*
* Issues Identified:
* 1. **State vs. Router**: Utilizes global state for navigation-related concerns better suited for URL routing, reducing shareability and persistence.
* 2. **Full List Reload**: Inefficiently reloads the entire list after adding an item, leading to unnecessary network use and performance hits.
* 3. **Global State Overuse**: Over-relies on global state for operations that could be localized, complicating the application unnecessarily.
* 1. State vs. Router: Utilizes global state for navigation-related concerns better suited for URL routing, reducing shareability and persistence.
* 2. Full List Reload: Inefficiently reloads the entire list after adding an item, leading to unnecessary network use and performance hits.
* 3. Global State Overuse: Over-relies on global state for operations that could be localized, complicating the application unnecessarily.
*
* Future Solutions:
* - **React Router Integration**: Leverage URL parameters for selecting and displaying binding constraints, improving UX and state persistence.
* - **React Query for State Management**: Utilize React Query for data fetching and state management. This introduces benefits like:
* - **Automatic Revalidation**: Only fetches data when needed, reducing unnecessary network requests.
* - **Optimistic Updates**: Immediately reflect changes in the UI while the backend processes the request, enhancing perceived performance.
* - **Cache Management**: Efficiently manage and invalidate cache, ensuring data consistency without manual reloads.
* - **Efficient State Updates**: Post-creation, append the new item to the existing list or use React Query's mutation to update the list optimally.
* - React Router Integration: Leverage URL parameters for selecting and displaying binding constraints, improving UX and state persistence.
* - React Query for State Management: Utilize React Query for data fetching and state management. This introduces benefits like:
* - Automatic Revalidation: Only fetches data when needed, reducing unnecessary network requests.
* - Optimistic Updates: Immediately reflect changes in the UI while the backend processes the request, enhancing perceived performance.
* - Cache Management: Efficiently manage and invalidate cache, ensuring data consistency without manual reloads.
* - Efficient State Updates: Post-creation, append the new item to the existing list or use React Query's mutation to update the list optimally.
*
* Adopting these strategies will significantly enhance efficiency, maintainability, and UX, addressing current architectural weaknesses.
*/
Expand Down Expand Up @@ -147,11 +146,18 @@ function AddDialog({
validateString(v, { existingValues: existingConstraints }),
}}
/>
{study.version >= "870" && (
{Number(study.version) >= 870 && (
<StringFE
name="group"
label={t("global.group")}
control={control}
rules={{
validate: (v) =>
validateString(v, {
max: 20,
specialChars: "-",
}),
}}
/>
)}
<StringFE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,40 @@ import { BindingConstraint } from "./BindingConstView/utils";
interface Props {
list: BindingConstraint[];
onClick: (name: string) => void;
currentBindingConst?: string;
currentConstraint?: string;
reloadConstraintsList: VoidFunction;
}

// TODO rename ConstraintsList
function BindingConstPropsView({
list,
onClick,
currentBindingConst,
currentConstraint,
reloadConstraintsList,
}: Props) {
const [bindingConstNameFilter, setBindingConstNameFilter] =
useState<string>();
const [searchedConstraint, setSearchedConstraint] = useState("");
const [addBindingConst, setAddBindingConst] = useState(false);
const [filteredBindingConst, setFilteredBindingConst] = useState<
BindingConstraint[]
>(list || []);
const [filteredConstraints, setFilteredConstraints] = useState(list);

useEffect(() => {
const filter = (): BindingConstraint[] => {
if (list) {
return list.filter(
(s) =>
!bindingConstNameFilter ||
s.name.search(new RegExp(bindingConstNameFilter, "i")) !== -1,
);
}
return [];
};
setFilteredBindingConst(filter());
}, [list, bindingConstNameFilter]);
if (!list) {
setFilteredConstraints([]);
return;
}

if (!searchedConstraint) {
setFilteredConstraints(list);
return;
}

const pattern = new RegExp(searchedConstraint, "i");
const filtered = list.filter((s) => pattern.test(s.name));

setFilteredConstraints(filtered);
}, [list, searchedConstraint]);

const existingConstraints = useMemo(
() => list.map(({ name }) => name.toLowerCase()),
() => list.map(({ name }) => name),
[list],
);

Expand All @@ -53,17 +53,17 @@ function BindingConstPropsView({
<PropertiesView
mainContent={
<ListElement
list={filteredBindingConst.map((item) => ({
label: item.name,
name: item.id,
list={filteredConstraints.map((constraint) => ({
label: constraint.name,
name: constraint.id,
}))}
currentElement={currentBindingConst}
currentElement={currentConstraint}
setSelectedItem={(elm) => onClick(elm.name)}
/>
}
secondaryContent={<div />}
onAdd={() => setAddBindingConst(true)}
onSearchFilterChange={(e) => setBindingConstNameFilter(e as string)}
onSearchFilterChange={(e) => setSearchedConstraint(e)}
/>
{addBindingConst && (
<AddDialog
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,28 @@ function Fields({ study, constraintId, isMatrixOpen, onCloseMatrix }: Props) {
rules={{ validate: (v) => validateString(v) }}
sx={{ m: 0 }} // TODO: Remove when updating MUI Theme
/>
<StringFE
name="group"
label={t("global.group")}
size="small"
control={control}
sx={{ m: 0 }} // TODO: Remove when updating MUI Theme
/>
{Number(study.version) >= 870 && (
<StringFE
name="group"
label={t("global.group")}
size="small"
control={control}
rules={{
validate: (v) =>
validateString(v, {
max: 20,
specialChars: "-",
}),
}}
sx={{ m: 0 }} // TODO: Remove when updating MUI Theme
/>
)}
<StringFE
name="comments"
label={t("study.modelization.bindingConst.comments")}
size="small"
control={control}
required={false}
sx={{ m: 0 }} // TODO: Remove when updating MUI Theme
/>
<SelectFE
Expand All @@ -110,8 +120,7 @@ function Fields({ study, constraintId, isMatrixOpen, onCloseMatrix }: Props) {
label={t("study.modelization.bindingConst.enabled")}
control={control}
/>

{study.version >= "840" && (
{Number(study.version) >= 840 && (
<Box sx={{ width: 1 }}>
<SelectFE
name="filterYearByYear"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function ConstraintTermItem({
event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>,
) => {
const value = event.target.value;
const newOffset = value === "" ? undefined : parseFloat(value);
const newOffset = value === "" ? undefined : parseInt(value, 10);
setOffset(newOffset);
saveValue({ ...term, offset: newOffset });
};
Expand Down Expand Up @@ -148,7 +148,7 @@ function ConstraintTermItem({
size="small"
type="number"
value={weight}
onChange={(e) => handleWeightChange(e)}
onChange={handleWeightChange}
sx={{ maxWidth: 150, mr: 0 }}
/>
}
Expand Down Expand Up @@ -184,7 +184,7 @@ function ConstraintTermItem({
size="small"
type="number"
value={offset}
onChange={(e) => handleOffsetChange(e)}
onChange={handleOffsetChange}
sx={{ maxWidth: 100 }}
/>
</OffsetInput>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ function Matrix({ study, operator, constraintId, open, onClose }: Props) {
),
};

////////////////////////////////////////////////////////////////
// JSX
////////////////////////////////////////////////////////////////

return (
<BasicDialog
contentProps={{
Expand All @@ -38,7 +42,7 @@ function Matrix({ study, operator, constraintId, open, onClose }: Props) {
maxWidth="xl"
{...dialogProps}
>
{study.version >= "870" ? (
{Number(study.version) >= 870 ? (
<>
{operator === "less" && (
<MatrixInput
Expand Down Expand Up @@ -74,7 +78,6 @@ function Matrix({ study, operator, constraintId, open, onClose }: Props) {
computStats={MatrixStats.NOCOL}
/>
</Box>

<Box sx={{ px: 2 }}>
<MatrixInput
study={study}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ function BindingConstView({ constraintId }: Props) {
response={mergeResponses(constraint, linksAndClusters)}
ifResolved={([defaultValues, linksAndClusters]) => (
<>
{/* Header controls */}
<Box
sx={{
pt: 1,
Expand Down Expand Up @@ -153,6 +154,7 @@ function BindingConstView({ constraintId }: Props) {
/>
</Box>
</Box>
{/* Constraint properties form */}
<Box sx={{ display: "flex", width: 1 }}>
<Form
config={{ defaultValues }}
Expand All @@ -167,6 +169,7 @@ function BindingConstView({ constraintId }: Props) {
/>
</Form>
</Box>
{/* Constraint terms form */}
<Box sx={{ display: "flex", flexGrow: 1 }}>
<Form autoSubmit config={{ defaultValues }} sx={{ flexGrow: 1 }}>
<BindingConstForm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,24 @@ function BindingConstraints() {
);

// TODO find better name
const constraintsRes = usePromise(
const constraints = usePromise(
() => getBindingConstraintList(study.id),
[study.id, bindingConstraints],
);

useEffect(() => {
if (constraintsRes.data && !currentConstraintId) {
const firstConstraintId = constraintsRes.data[0].id;
if (constraints.data && !currentConstraintId) {
const firstConstraintId = constraints.data[0].id;
dispatch(setCurrentBindingConst(firstConstraintId));
}
}, [constraintsRes, currentConstraintId, dispatch]);
}, [constraints, currentConstraintId, dispatch]);

////////////////////////////////////////////////////////////////
// Event Handlers
////////////////////////////////////////////////////////////////

const handleConstraintChange = (bindingConstId: string): void => {
dispatch(setCurrentBindingConst(bindingConstId));
const handleConstraintChange = (constraintId: string): void => {
dispatch(setCurrentBindingConst(constraintId));
};

////////////////////////////////////////////////////////////////
Expand All @@ -55,16 +55,16 @@ function BindingConstraints() {

return (
<UsePromiseCond
response={constraintsRes}
response={constraints}
ifPending={() => <SimpleLoader />}
ifResolved={(data) => (
<SplitView direction="horizontal" sizes={[10, 90]} gutterSize={3}>
<Box>
<BindingConstPropsView // TODO rename ConstraintsList
list={data}
onClick={handleConstraintChange}
currentBindingConst={currentConstraintId}
reloadConstraintsList={constraintsRes.reload}
currentConstraint={currentConstraintId}
reloadConstraintsList={constraints.reload}
/>
</Box>
<Box>
Expand Down

0 comments on commit 09f2865

Please sign in to comment.