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 revert changes button in local unit form #1588

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

shreeyash07
Copy link
Collaborator

@shreeyash07 shreeyash07 commented Dec 15, 2024

Addresses:

Depends on:

Changes

  • Add revert changes button in local unit form
  • Add revert changes request
  • Add revert changes modal form

This PR doesn't introduce:

  • typos
  • conflict markers
  • unwanted comments
  • temporary files, auto-generated files or secret keys
  • console.log meant for debugging
  • codegen errors

Sorry, something went wrong.

Copy link

changeset-bot bot commented Dec 15, 2024

⚠️ No Changeset found

Latest commit: d729c79

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@barshathakuri barshathakuri self-requested a review December 16, 2024 04:38
{readOnly && isDefined(actionsContainerRef.current) && (
{!localUnitDetailsResponse?.is_locked
&& readOnlyFromProps
&& isNotDefined(localUnitDetailsResponse)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be isDefined(localUnitDetailsResponse) ?
Also, this check should come before !localUnitDetailsResponse?.is_locked

@@ -592,6 +660,15 @@ function LocalUnitsForm(props: Props) {
&& (environment !== 'production')
&& (
<div className={styles.actions}>
{localUnitDetailsResponse?.is_locked && (
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop optional chaining.

Also, shouldn't the logic be inverted? Do we show "revert" button if the localUnit is locked?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, we do show revert button if localunit is locked

@shreeyash07 shreeyash07 requested a review from tnagorra December 16, 2024 11:28
@samshara samshara force-pushed the feature/changes-modal branch 2 times, most recently from 2aefe40 to 153f5c4 Compare December 17, 2024 07:49
@samshara samshara force-pushed the feature/revert-changes branch from 3ea145e to f04c03b Compare December 17, 2024 14:14
@samshara samshara force-pushed the feature/changes-modal branch from 36763c9 to 0c4bffc Compare December 17, 2024 14:20
Base automatically changed from feature/changes-modal to project/local-unit-v2 December 18, 2024 05:29

Verified

This commit was signed with the committer’s verified signature.
samshara Sameer Shakten Rai

Verified

This commit was signed with the committer’s verified signature.
samshara Sameer Shakten Rai
@samshara samshara force-pushed the feature/revert-changes branch from f04c03b to 87ac5d5 Compare December 18, 2024 05:38
@samshara samshara self-requested a review December 18, 2024 05:41

Verified

This commit was signed with the committer’s verified signature.
samshara Sameer Shakten Rai
@samshara samshara merged commit 285dbd7 into project/local-unit-v2 Dec 18, 2024
9 of 10 checks passed
@samshara samshara deleted the feature/revert-changes branch December 18, 2024 06:16
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.

None yet

4 participants