Skip to content
Snippets Groups Projects

Move General tasks and icons to the frontend

Merged Giannis Koumoutsos requested to merge 148-general_category_in_frontend into main

Closes #148 (closed)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M12 8v-5l9 9-9 9v-5h-12v-8h12zm12-4h-3v16h3v-16z"/></svg>
    • Note that this is all up for discussion but I don't think trying to hijack the tasks fetching mechanism to add makeshift tasks is the right approach.

      I would advise to implement these "general" tasks as Note nodes and Subworkflow nodes: in specific components that behave like task items in the task list.

      For this I would advise to extract the category "General" (!393 (diffs)) in a specific component and to create new components for each "General" task

    • It seems cleaner the way you propose but it was too complicated to handle them exactly as notes due to the icon used for each. So I handle them as tasks but only in the taskList and I do not add them to the tasks from the server.

    • Please register or sign in to reply
  • added 1 commit

    • 4c8fd95e - not pollute tasks when getting them from server

    Compare with previous version

  • 42 task={task}
    43 onClick={() => setSelectTaskId(task.task_identifier)}
    44 isSelected={task.task_identifier === selectedTaskId}
    45 />
    46 ))}
    47 {category === 'General' && (
    48 <>
    49 <AddNoteButton />
    50 <AddSubworkflow />
    51 </>
    52 )}
    53 </div>
    54 </AccordionDetails>
    55 </Accordion>
    56 ))}
    28 {[...new Set(tasks.map((m) => m.category)).values(), 'General'].map(
    • In this line, you add artifically General to the list of categories but this forces you to introduce code below that is specific to the General category.

      As I told you in my previous comment, I think it would be cleaner to abstract to General category in a dedicated component. Sure, you will need to duplicate the Accordion structure but this will be so much easier to read and maintain.

    • Giannis Koumoutsos changed this line in version 3 of the diff

      changed this line in version 3 of the diff

    • Please register or sign in to reply
  • 4 4
    5 5 import styles from './TaskList.module.css';
    6 6
    7 function AddNoteButton() {
    7 function AddGeneralNodeButton() {
  • 4 4
    5 5 import styles from './TaskList.module.css';
    6 6
    7 function AddNoteButton() {
    7 function AddGeneralNodeButton() {
    8 8 return (
    9 <Tooltip title="Drag to the canvas to add a note node" arrow>
    9 <Tooltip title="Drag to the canvas to add an note node" arrow>
  • 163 163
    164 164 let task: Task | undefined;
    165 165
    166 if (task_type !== 'note') {
    166 if (
    167 !['note', 'graphInput', 'graphOutput', 'taskSkeleton'].includes(
    168 task_identifier
    169 )
  • 5 icon: 'graphInput.svg',
    6 task_type: 'graphInput',
    7 output_names: [''],
    8 task_identifier: 'graphInput',
    9 optional_input_names: [''],
    10 category: 'General',
    11 required_input_names: [''],
    12 },
    13 {
    14 icon: 'graphOutput.svg',
    15 task_type: 'graphOutput',
    16 output_names: [''],
    17 task_identifier: 'graphOutput',
    18 optional_input_names: [''],
    19 category: 'General',
    20 required_input_names: [''],
  • 16 output_names: [''],
    17 task_identifier: 'graphOutput',
    18 optional_input_names: [''],
    19 category: 'General',
    20 required_input_names: [''],
    21 },
    22 {
    23 required_input_names: [],
    24 category: 'General',
    25 task_identifier: 'taskSkeleton',
    26 task_type: 'ppfmethod',
    27 icon: 'orange2.png',
    28 optional_input_names: [],
    29 output_names: [],
    30 },
    31 ];
  • Good progress ! I think we can still improve it through.

  • added 1 commit

    Compare with previous version

  • 177 171 output_names: undefined,
    178 172 required_input_names: undefined,
    179 173 };
  • 43 </AccordionSummary>
    44 <AccordionDetails>
    45 <div className={styles.itemContainer}>
    46 {generalTasks.map((task) => (
    47 <TaskItem key={task.task_identifier} task={task} />
    48 ))}
    49
    50 <AddNoteButton />
    51 <AddSubworkflow />
    52 </div>
    53 </AccordionDetails>
    54 </Accordion>
    55 );
    56 }
    57
    58 export default GeneralTasksList;
  • Nice !

    Can you add a Cypress test that tests that the "General" category is present in the task drawer and that it contains the related tasks ?

  • Loic Huder approved this merge request

    approved this merge request

  • added 1 commit

    • fcc6fbea - added tests for general nodes

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading