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

[7122] Workflow is not cancelled when renaming an item's parent #4380

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

jvega190
Copy link
Member

@jvega190 jvega190 requested a review from rart as a code owner February 25, 2025 15:28
Copy link
Member

@rart rart left a comment

Choose a reason for hiding this comment

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

  • Review, understand (ask questions) and test the changes I've pushed
  • One scenario I'm not too clear on how it works in practice is the suggested modified value being a path that exists. Please test and lets review that scenario

@jvega190
Copy link
Member Author

jvega190 commented Feb 26, 2025

Reviewed changes, tested the different scenarios.

@jvega190 jvega190 requested a review from rart February 26, 2025 18:19
@jvega190
Copy link
Member Author

@rart I updated the workflow warning message to be the same as in ViewPackagesDialog (The item is part of one or more publishing packages. Editing it will cancel the packages.). Though since this scenario is a bit different (the parent has children in workflow, instead of being the item in workflow), I',m not sure it applies. What do you thing?

<PrimaryButton onClick={onSubmit} disabled={isSubmitting || !isValid} loading={isSubmitting}>
<PrimaryButton
onClick={onSubmit}
disabled={isSubmitting || !isValid || fetchingAffectedPackages}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
disabled={isSubmitting || !isValid || fetchingAffectedPackages}
disabled={!isValid}

Copy link
Member Author

Choose a reason for hiding this comment

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

We would lose the isSubmitting validation, should I keep it here or add it to the isValid var?

Copy link
Member

Choose a reason for hiding this comment

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

isSubmitting is considered by the loading prop of the button

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.

2 participants