From 22e537c6b490d97e142aa25f6d09da38037ba555 Mon Sep 17 00:00:00 2001 From: Paul-Hammer <65820985+Paul-Hammer@users.noreply.github.com> Date: Wed, 23 Oct 2024 13:32:11 +0300 Subject: [PATCH 1/4] Update Settings.tsx --- src/components/settings/Settings.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/settings/Settings.tsx b/src/components/settings/Settings.tsx index a09a42e0..5e9511af 100644 --- a/src/components/settings/Settings.tsx +++ b/src/components/settings/Settings.tsx @@ -11,14 +11,19 @@ interface SettingsProps { onChange: (options: VoyagerDisplayOptions) => void; } +// What is the reason to export this component by default? export default function Settings(props: SettingsProps) { + // Why you avoid destructuring component's parameters and doing it inside component's body? const { typeGraph, options, onChange } = props; + // Move this outside of the component. No need to run all this code, including imports, if "typegraph == null". if (typeGraph == null) { return null; } return ( { ... } sx={(theme) => ({ position: 'absolute', opacity: 1, @@ -29,6 +34,7 @@ export default function Settings(props: SettingsProps) { borderColor: theme.palette.shadowColor.main, boxShadow: 2, padding: 1, + // Obvious and useless hint, agree? // left-bottom corner left: 0, bottom: 0, @@ -39,6 +45,8 @@ export default function Settings(props: SettingsProps) { onChange={(rootType) => onChange({ rootType })} /> + {/* 4 similar checkboxes with labels. Create a data structure (Checkbox[]) and loop through its items rendering Checkbox + label on each step. + Use "useMemo" for this data structure to avoid problems with re-renders. */} Date: Wed, 23 Oct 2024 13:50:28 +0300 Subject: [PATCH 2/4] Update Settings.tsx --- src/components/settings/Settings.tsx | 84 +++++++++++++--------------- 1 file changed, 39 insertions(+), 45 deletions(-) diff --git a/src/components/settings/Settings.tsx b/src/components/settings/Settings.tsx index 5e9511af..8985e881 100644 --- a/src/components/settings/Settings.tsx +++ b/src/components/settings/Settings.tsx @@ -1,3 +1,4 @@ +import { ChangeEvent, Fragment, useMemo } from "react"; import Checkbox from '@mui/material/Checkbox'; import Stack from '@mui/material/Stack'; @@ -11,19 +12,39 @@ interface SettingsProps { onChange: (options: VoyagerDisplayOptions) => void; } -// What is the reason to export this component by default? -export default function Settings(props: SettingsProps) { - // Why you avoid destructuring component's parameters and doing it inside component's body? - const { typeGraph, options, onChange } = props; - // Move this outside of the component. No need to run all this code, including imports, if "typegraph == null". - if (typeGraph == null) { - return null; - } +export default function Settings({ typeGraph, options, onChange }: SettingsProps) { + const checkboxes = useMemo( + () => [ + { + id: "sort", + label: "Sort by Alphabet", + checked: options.sortByAlphabet ?? false, + onChange: (e: ChangeEvent) => onChange({ sortByAlphabet: e.target.checked }), + }, + { + id: "skip", + label: "Skip Relay", + checked: options.skipRelay ?? false, + onChange: (e: ChangeEvent) => onChange({ skipRelay: e.target.checked }), + }, + { + id: "deprecated", + label: "Skip deprecated", + checked: options.skipDeprecated ?? false, + onChange: (e: ChangeEvent) => onChange({ skipDeprecated: e.target.checked }), + }, + { + id: "showLeafFields", + label: "Show leaf fields", + checked: options.showLeafFields ?? false, + onChange: (e: ChangeEvent) => onChange({ showLeafFields: e.target.checked }), + }, + ], + [options, onChange] + ); return ( { ... } sx={(theme) => ({ position: 'absolute', opacity: 1, @@ -34,8 +55,6 @@ export default function Settings(props: SettingsProps) { borderColor: theme.palette.shadowColor.main, boxShadow: 2, padding: 1, - // Obvious and useless hint, agree? - // left-bottom corner left: 0, bottom: 0, })} @@ -44,39 +63,14 @@ export default function Settings(props: SettingsProps) { typeGraph={typeGraph} onChange={(rootType) => onChange({ rootType })} /> - - {/* 4 similar checkboxes with labels. Create a data structure (Checkbox[]) and loop through its items rendering Checkbox + label on each step. - Use "useMemo" for this data structure to avoid problems with re-renders. */} - - onChange({ sortByAlphabet: event.target.checked }) - } - /> - - onChange({ skipRelay: event.target.checked })} - /> - - - onChange({ skipDeprecated: event.target.checked }) - } - /> - - - onChange({ showLeafFields: event.target.checked }) - } - /> - + + + {checkboxes.map(({ id, checked, label, onChange }) => ( + + + + + ))} ); From 9c48fb7547bb3c4645d16a8c5a4cc9e767995eac Mon Sep 17 00:00:00 2001 From: Paul-Hammer <65820985+Paul-Hammer@users.noreply.github.com> Date: Wed, 23 Oct 2024 13:59:02 +0300 Subject: [PATCH 3/4] Update Settings.tsx --- src/components/settings/Settings.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/settings/Settings.tsx b/src/components/settings/Settings.tsx index 8985e881..5688f7d8 100644 --- a/src/components/settings/Settings.tsx +++ b/src/components/settings/Settings.tsx @@ -7,7 +7,7 @@ import { VoyagerDisplayOptions } from '../Voyager'; import RootSelector from './RootSelector'; interface SettingsProps { - typeGraph: TypeGraph | null; + typeGraph: TypeGraph; options: VoyagerDisplayOptions; onChange: (options: VoyagerDisplayOptions) => void; } From ec69f2eacbf82d00c2b0ced5abe24921f8220774 Mon Sep 17 00:00:00 2001 From: Paul-Hammer <65820985+Paul-Hammer@users.noreply.github.com> Date: Wed, 23 Oct 2024 14:05:31 +0300 Subject: [PATCH 4/4] Update Voyager.tsx --- src/components/Voyager.tsx | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/Voyager.tsx b/src/components/Voyager.tsx index cfd0115e..49016237 100644 --- a/src/components/Voyager.tsx +++ b/src/components/Voyager.tsx @@ -191,23 +191,25 @@ export default function Voyager(props: VoyagerProps) { } function renderGraphViewport() { + const showSettings = !hideSettings && typeGraph != null; + return ( ({ flex: 1, - position: 'relative', - display: 'inline-block', - width: '100%', - height: '100%', - maxHeight: '100%', - - [theme.breakpoints.down('md')]: { - height: '50%', - maxWidth: 'none', + position: "relative", + display: "inline-block", + width: "100%", + height: "100%", + maxHeight: "100%", + + [theme.breakpoints.down("md")]: { + height: "50%", + maxWidth: "none", }, })} > - {!hideSettings && ( + {showSettings && (