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 FormSteps React component #3952

Merged
merged 6 commits into from Jul 24, 2020
Merged

Add FormSteps React component #3952

merged 6 commits into from Jul 24, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 20, 2020

Why: To manage common behaviors of form values aggregation and history of a linear, step-based form.

Screen Recording:

form-steps mov

The included example is provided for demonstration only, and is not intended to be styled or representative of a final flow. The intent of these changes is to prepare an apparatus for step-based forms flow which accumulates a single payload object, and which can be revised by the user using their browser history.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

**Why**: History#pushState cannot operate on origin about:blank, the default JSDOM URL
**Why**: setupDocumentCaptureTestDOM is called before _each_ test case within the Acuant tests, whereas the tear down is only called once at the end of the test suite. This has the result that assigning originalWindow in beforeEach may in-fact be tracking the newly-created Acuant DOM as the original. The alternative to this is to tear down after each test case. However, with recent changes, it's assumed that the DOM should be ready to use at import-time, such that this should be safe.
**Why**: To manage common behaviors of form values aggregation and history of a linear, step-based form.
**Why**: These texts are unlikely to change and are already readily available.

See: #3952 (comment)
**Why**: Airbnb doesn't document this rule. It's assumed the spirit of the intent of this rule is to ensure consistency of one element per line when splitting elements across multiple lines. This is accounted for by Prettier. However, the rule is more strict in how it allows multiple elements within the same line, in a way which Prettier neither applies, nor does Airbnb prescribe, nor is in contradiction to this spirit of the guideline.
@aduth
Copy link
Member Author

aduth commented Jul 24, 2020

Per the discussions at #3952 (comment) and #3952 (comment), I took another shot at extracting a general-purpose useHistoryParam in 8332920, and updated the FormSteps component to work from the value of the name parameter as the source of truth, finding the appropriate step from that name.

A couple notes:

  • As a reasonable compromise on the topic of browser support raised in Add FormSteps React component #3952 (comment), I chose to implement a limited polyfill of the behavior we would expect from URLSearchParams#get to retrieve a parameter value
  • I revised the implementation to assign the parameter to the URL hash fragment instead of the URL query string. This was partly to facilitate testing (Error: Not implemented: navigation  jsdom/jsdom#2112), but generally speaking I think it's more common to see pushState used to modify hash fragments than it is to see them used to modify query parameters, so it could be an improvement all the same

@aduth
Copy link
Member Author

aduth commented Jul 24, 2020

Updated screen recording, since a few other pull requests have complemented the effort here toward a more realistic submission experience:

final-form-steps mov

@aduth aduth merged commit d4cefa7 into master Jul 24, 2020
@aduth aduth deleted the aduth-form-steps branch July 24, 2020 15:08
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

2 participants