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

feat: modal form for reporting inaccurate Planning Constraints #3436

Merged
merged 13 commits into from
Jul 23, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Jul 17, 2024

Changes:

  • Adds a new feature flag "OVERRIDE_CONSTRAINTS" so we can work on followup PRs in parallel
  • Adds a link & modal to capture user input about inaccurate constraints (on intersecting constraints only):
    • The modal has context text, a checklist of entities that we think apply, and a long text input to provide a reason why it's inaccurate
    • "Submit" button does validation and updates internal component state with valid input
    • "Cancel" button clears input and exits the modal
  • Basic styling feedback if you've marked an entity as inaccurate: gray text and "[Not applicable]" tag next to entity, and overall gray text accordion title if every entity within that category has been marked as inaccurate

Followup PRs:

  • Styling questions for Ian:
    • There's a mix of internal & external links in each accordion drawer, how best to distinguish?
    • More polished visual feedback after marking a constraint as inaccurate
    • Modal accessibility (currently re-using a lot of configs of UploadAndLabel modal)
  • Passport logic for overall component onSubmit & automations
  • TESTS !

Testing thread here: https://opensystemslab.slack.com/archives/C04DZ1NBUMR/p1721219483725279

Copy link

github-actions bot commented Jul 17, 2024

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak marked this pull request as ready for review July 22, 2024 08:35
@jessicamcinchak jessicamcinchak requested a review from a team July 22, 2024 08:41
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

A few minor comments and suggestions 🙂

setTextInput(e.target.value);
};

const validateAndSubmit = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

One edge-case workflow -

  • I mark a constraint as not applying
  • I re-open modal to uncheck
  • I can't save without error
  • I have to cancel to get back to no overrides

This ties into the comment above on cancel behaviour - I think it is valid to submit this form with nothing checked, if the form is dirty.

Copy link
Member Author

@jessicamcinchak jessicamcinchak Jul 22, 2024

Choose a reason for hiding this comment

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

Can recreate and think I've got a solution for this one:

  • if you open an empty form and click "Submit", I'll still trigger the validation error messages and prevent submission
  • if you open a pre-populated form, clear your selected options and text input, and click "Submit" - I'll sync your updates to parent state (essentially "clear") and close the modal
    • is it obvious you have to manually clear both your selected options and text input? Would you expect text input to auto-clear as soon as zero checklist options are selected? (getting into conditional question territory here)

Copy link
Contributor

@DafyddLlyr DafyddLlyr Jul 23, 2024

Choose a reason for hiding this comment

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

Working as expected!

One (very) minor issue is that errors persist from the modal being closed and opened - I guess they should be cleared on both submit and cancel?

@jessicamcinchak jessicamcinchak merged commit afd5857 into main Jul 23, 2024
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/override-modal branch July 23, 2024 10: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.

2 participants