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: nested error discovery in check form #868

Merged
merged 81 commits into from
Jul 18, 2024

Conversation

ckbedwell
Copy link
Contributor

@ckbedwell ckbedwell commented Jul 16, 2024

Problem

As noted in the check form redesign work, this is required to help with error discovery: #847

With the wizard form pattern and now with the request components, it means that users may have difficulty finding errors to correct when submitting check creation / updates.

Solution

Add methods and event listeners which respond to an invalid check form submission. Because of the nature of the individual components which need to respond to the invalid submission it doesn't make sense to rearchitect them to respond via prop drilling or some overarching context. I have chosen to do this a little bit more vanilla with dispatching a custom event, setting up event listeners for the custom event and utilising useImperativeHandle where appropriate.

I have added test coverage for each of the check types which is doing a single spot check that the field with an error is being navigated to. I did have checks ensuring the input field was receiving focus but I have had to remove them as they were proving a little flaky in the CI/CD. I have left two specific focus checks for the script and probe fields as they are special and need to handled differently to all other form inputs.

Http check error discovery

Screen.Recording.2024-07-17.at.11.39.48.mov

Multihttp check error discovery

Screen.Recording.2024-07-17.at.11.44.00.mov

Script check error discovery, redirecting back to script tab from examples

Screen.Recording.2024-07-17.at.11.45.25.mov

Focussing probes select correctly when it has an error and is the first field

Screen.Recording.2024-07-17.at.11.46.59.mov

ckbedwell and others added 30 commits May 3, 2024 17:30
@ckbedwell ckbedwell mentioned this pull request Jul 17, 2024
@ckbedwell ckbedwell marked this pull request as ready for review July 17, 2024 10:48
@ckbedwell ckbedwell requested a review from a team as a code owner July 17, 2024 10:48
@ckbedwell ckbedwell requested a review from VikaCep July 17, 2024 10:48
Copy link
Contributor

@VikaCep VikaCep left a comment

Choose a reason for hiding this comment

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

👏 👏 👏 👏

src/components/CheckForm/checkForm.hooks.ts Outdated Show resolved Hide resolved
@ckbedwell ckbedwell requested a review from VikaCep July 18, 2024 12:03
VikaCep
VikaCep previously approved these changes Jul 18, 2024
Base automatically changed from feat/single-page-check-creation to main July 18, 2024 14:53
@ckbedwell ckbedwell dismissed VikaCep’s stale review July 18, 2024 14:53

The base branch was changed.

@ckbedwell ckbedwell requested a review from VikaCep July 18, 2024 15:19
@ckbedwell ckbedwell merged commit 316ef14 into main Jul 18, 2024
5 checks passed
@ckbedwell ckbedwell deleted the feat/check-page-nested-error-discovery branch July 18, 2024 15:22
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