Refactor tasks into components #470
Conversation
| pathname: "/evaluate/rank_initial_prompts", | ||
| type: "rank_initial_prompts", | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
it would make more sense to split this array into 2.
One for create and one for evaluate.
category could be an enum.
There was a problem hiding this comment.
Assuming we will have more than 2 categories, this makes it future proof. Plus, this is array is used to determine which category of component to render for random.
Regarding enum good idea, will do.
| <div className={`p-12 ${mainBgClasses}`}> | ||
| <TwoColumnsWithCards> | ||
| <> | ||
| <h5 className="text-lg font-semibold">Reply as the assistant</h5> |
There was a problem hiding this comment.
I am assuming this still needs to be updated right?
There was a problem hiding this comment.
Correct. On it.
Edit: added those to the TaskType and loading them from there now.
| export const Task = ({ tasks, trigger, mutate, mainBgClasses }) => { | ||
| const task = tasks[0].task; | ||
|
|
||
| function taskTypeComponent(type) { | ||
| const category = TaskTypes.find((taskType) => taskType.type === type).category; | ||
|
|
||
| switch (category) { | ||
| case "create": | ||
| return <CreateTask tasks={tasks} trigger={trigger} mutate={mutate} mainBgClasses={mainBgClasses} />; | ||
| case "evaluate": | ||
| return <EvaluateTask tasks={tasks} trigger={trigger} mutate={mutate} mainBgClasses={mainBgClasses} />; | ||
| } | ||
| } | ||
|
|
||
| return taskTypeComponent(task.type); | ||
| }; |
There was a problem hiding this comment.
nit:
This component adds an extra level of indirection that is not necessary.
I would argue that you can directly use CreateTask or EvaluateTask in the corresponding task page.
That being said, I understand its value for the random task type.
There was a problem hiding this comment.
The reason I make a unified component and also combined everything into one array is because I'm assuming we will have more tasks in future, so we will need to know how to handle any type, given the response from the backend.
|
This looks pretty good. I'll approve and merge after you've marked it ready for review and comment on or address @AbdBarho's comments. |
AbdBarho
requested review from
fozziethebeat and
k-nearest-neighbor
as code owners
Another draft for refactoring the tasks into components. Related to #433