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

Introduces a better Modal popup for unsaved changes in Forms. #2285

Merged
merged 11 commits into from
Aug 14, 2024

Conversation

deepakjosp
Copy link
Contributor

@deepakjosp deepakjosp commented Aug 13, 2024

Fixes https://github.com/bigbinary/neeto-engineering-web/issues/732

Description
blocknavigation-changes

Checklist

  • I have made corresponding changes to the documentation.
  • I have updated the types definition of modified exports.
  • I have verified the functionality in some of the neeto web-apps.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added proper data-cy and data-testid attributes.
  • I have added the necessary label (patch/minor/major - If package publish
    is required).

Reviewers

@neetodeploy neetodeploy bot temporarily deployed to neeto-ui-kpyu-pr-2285 August 13, 2024 07:16 Inactive
@deepakjosp deepakjosp self-assigned this Aug 13, 2024
@neetodeploy neetodeploy bot temporarily deployed to neeto-ui-kpyu-pr-2285 August 13, 2024 09:31 Inactive
… validations and refactored the queries and added one more extra field in the form.
…d continue is cled when there's an error in the form.
@neetodeploy neetodeploy bot temporarily deployed to neeto-ui-kpyu-pr-2285 August 13, 2024 09:56 Inactive
@deepakjosp deepakjosp added the patch Releases small requests or bug fixes. label Aug 13, 2024
@deepakjosp
Copy link
Contributor Author

@AbhayVAshokan _a please review.

@neetogit-bot neetogit-bot bot assigned AbhayVAshokan and unassigned deepakjosp Aug 13, 2024
Copy link
Member

@AbhayVAshokan AbhayVAshokan left a comment

Choose a reason for hiding this comment

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

Functionality looks great.

@praveen-murali-ind _a: I think having two primary action buttons is too much. Can we convert one of them to another style? WDYT?

Current UI: style: danger

image


Other possibilities

Style: danger-text

image

Style: secondary

image

Style tertiary

image

Style: text

image

Copy link
Contributor

@praveen-murali-ind praveen-murali-ind left a comment

Choose a reason for hiding this comment

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

LGTM. @AbhayVAshokan In this case, each button represents a distinct primary action. For consistency across Neeto, we typically use solid red buttons for danger actions.

@praveen-murali-ind praveen-murali-ind merged commit 0a1ea42 into main Aug 14, 2024
1 check passed
@praveen-murali-ind praveen-murali-ind deleted the 732-a-better-modalpop-for-unsaved-changes branch August 14, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Releases small requests or bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants