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

Add delete button to modify view #344

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

raviks789
Copy link
Contributor

@raviks789 raviks789 commented Aug 5, 2022

The modify view for the process form and edit node form must contain delete button. This allows the user to either
store the modifications or delete the nodes.

ref #253

@raviks789 raviks789 added the enhancement New feature or improvement label Aug 5, 2022
@raviks789 raviks789 requested a review from nilmerg August 5, 2022 12:24
@raviks789 raviks789 self-assigned this Aug 5, 2022
@cla-bot cla-bot bot added the cla/signed label Aug 5, 2022
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

  • The removal is still possible in tile view
  • Editing a non-process node doesn't allow removal yet

application/forms/ProcessForm.php Outdated Show resolved Hide resolved
@nilmerg nilmerg linked an issue Aug 29, 2022 that may be closed by this pull request
@nilmerg nilmerg added this to the 2.5.0 milestone Aug 29, 2022
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch from b7920fc to 4702eab Compare September 1, 2022 10:51
@raviks789 raviks789 requested a review from nilmerg September 1, 2022 10:52
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch from 4702eab to d718311 Compare September 1, 2022 10:55
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch from d718311 to fee17e1 Compare September 20, 2022 09:51
@raviks789 raviks789 requested a review from nilmerg September 20, 2022 09:52
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch 2 times, most recently from 448da77 to 0df5226 Compare July 24, 2023 11:49
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch 3 times, most recently from 60b5622 to 6a0e373 Compare July 24, 2023 13:14
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
application/forms/EditNodeForm.php Outdated Show resolved Hide resolved
application/forms/ProcessForm.php Outdated Show resolved Hide resolved
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch from 6a0e373 to 468f277 Compare July 24, 2023 14:06
@sukhwinder33445 sukhwinder33445 self-requested a review July 25, 2023 08:14
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

The new Delete element only redirects, so why not just use an anchor button?

The modify view for the process form and edit node form must contain delete button. This allows the user to either
store the modifications or delete the nodes.
Imported nodes will not have edit icon on their node tile. Hence, it is required to show the delete icon in their node tile.
As the other nodes will have edit icon and their edit forms contain the delete button, it is not necessary to show the
delete/cancel icon in their node tiles.
@raviks789 raviks789 force-pushed the feature/delete-button-in-modify-view branch from 468f277 to 5a18138 Compare July 25, 2023 13:39
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

Shouldn't the delete button have some style @nilmerg @flourish86 ? I'm thinking of delete buttons in icingadb-web.

@nilmerg nilmerg removed this from the 2.5.0 milestone Aug 3, 2023
@flourish86
Copy link
Contributor

Shouldn't the delete button have some style @nilmerg @flourish86 ? I'm thinking of delete buttons in icingadb-web.

Looks fine to me this way.

Screen Shot 2023-08-08 at 16 04 39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete button for modify view
4 participants