feat: add toggles to Input Panel#14
Conversation
src/components/InputPanel.tsx
Outdated
| import CodeEditor from './CodeEditor'; | ||
| import { ReactComponent as PlusIcon } from '../images/plus.svg'; | ||
| import { ReactComponent as DownArrow } from '../images/down-arrow.svg'; | ||
| import { useCallback, useContext, useEffect, useMemo, useState } from "react"; |
There was a problem hiding this comment.
Our team prefers using single quotes. Can you revert these quote changes please?
src/components/InputPanel.tsx
Outdated
| className={`${styles.panelHeaderButton} ${ | ||
| view === viewType ? styles.panelHeaderButtonSelected : "" | ||
| }`} | ||
| onClick={changeView.bind(null, viewType as ViewType)} |
There was a problem hiding this comment.
I like what you've done here to simplify the code as we add more buttons. However, I generally try to avoid using .bind() because it returns a new function on every render which can cause extra re-rendering down the line. Maybe it's time to create a ViewTypeButton component to which you pass a viewType and an onSelected prop which expects the viewType as a first argument.
src/components/InputPanel.tsx
Outdated
| return ( | ||
| <div | ||
| className={styles.inputItem} | ||
| onClick={setSelectedInputFileIndex.bind(null, index)} |
There was a problem hiding this comment.
Example here of a bind that I should refactor
| }; | ||
| type ViewType = keyof typeof views; | ||
|
|
||
| const functionList = [ |
There was a problem hiding this comment.
Nice! I like the direction. I think each listed function should have a name, description, and list of arguments. Probably here in the left panel you would just see the name, then hovering over them or clicking a "more info" button would display an info box with the description and list of arguments.
We don't need the actual function implementations here because they are implemented in @comake/rmlmapper-js. However, we do want to allow users to add custom functions for which we will need to save their code.
src/components/InputPanel.tsx
Outdated
|
|
||
| function InputPanel({ addNewInput }: InputPanelProps) { | ||
| const [view, setView] = useState(views.inputs); | ||
| const [view, setView] = useState<ViewType>("inputs"); |
There was a problem hiding this comment.
I don't love that this is now using a copied string rather than a referenced value (eg. referencing a key of views). Maybe we need to create two maps, one called views and one called viewLabels:
const views = {
inputs: 'inputs',
functions: 'functions',
};
const viewLabels = {
[views.inputs]: 'Input Files',
[views.functions]: 'Functions',
};This was you can initialize state with views.inputs and use viewLabels[view] for the label.
|
I have pushed the updates but some github services seem to not be working at the moment. Let me know if you have any issues with viewing my changes. |
|
Closed on favor of working in one repo in #19 |
2 participants
This PR will close #9