Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust copy and delete #174

Merged
merged 13 commits into from
Dec 15, 2023
Merged

Adjust copy and delete #174

merged 13 commits into from
Dec 15, 2023

Conversation

winniel24
Copy link
Contributor

Summary

  1. Adjusted process-delete module and process-copy module
  • Removed don't-ask-again option (user-preferences and checkbox)
  • Removed single-delete component
  • Implemented Scrollbar component in copy-modal
  • Removed option to keep multiple editing tabs open in copy-modal (only one open at a time)
  1. Added copy function to process-list's action bar and replaced process-delete module in processes.tsx and process-list.tsx with pop-confirmations --> resulting in error for both delete and copy (todo!)
    image

  2. Adjusted mobile view for screensize <= sm

  • Hide side-bar
  • Make PROCEED icon appear in header
  • Hide process metadata

@FelipeTrost
Copy link
Contributor

Popconfirm should be replaced by confirmation-button, for which you have to wait until this pr is merged.

type="text"
style={{ marginTop: '20px', marginLeft: '15px' }}
icon={<MenuOutlined style={{ fontSize: '170%' }} />}
/>
onClick={() => !siderOpened}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the onClick Function do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the onClick function as it's not used anymore

});
});
setSelection([]);
}, [deleteProcess, selection]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing dependency: setSelection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing dependency was added


<AuthCan action="delete" resource={toCaslResource('Process', process)}>
<Tooltip placement="top" title={'Delete'}>
<Popconfirm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use ConfirmationButton instead of PopConfirm

@@ -103,6 +112,20 @@ const ProcessList: FC<ProcessListProps> = ({
onSettled: refreshData,
});

const deleteSelectedProcesses = useCallback(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could move this function to processes and use it together with ConfirmationButton, then you could delete the process-delete components

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used deleteSelectedProcesses function from processes.tsx and declared its type as "Dispatch<SetStateAction<Key[]>>" in process-list.tsx. However, this declaration shows the error "Expected 1 arguments, but got 0." Would there be a better type to declare it as?

@OhKai
Copy link
Contributor

OhKai commented Dec 11, 2023

Once @FelipeTrost is happy with his review, I can take care of the merge conflicts with main, since I changed a lot with the switch to Server Components.

Copy link
Contributor

@OhKai OhKai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged it with my server components/actions changes and simplified a lot of the code structure. All mutations now happen in processes.tsx

@OhKai OhKai merged commit f476e0c into main Dec 15, 2023
11 checks passed
@OhKai OhKai deleted the adjust-copy-and-delete branch December 15, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants