Mock api / Throwaway Edit UI Changes #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jimmoffet
commented
Feb 26, 2024
| @@ -118,12 +119,12 @@ const SequenceElementEdit: FormElementComponent<SequenceElement> = ({ | |||
| <input type="hidden" {...register(`${element.id}.id`)} /> | |||
| <input type="hidden" {...register(`${element.id}.type`)} /> | |||
| <input type="hidden" {...register(`${element.id}.elements`)} /> | |||
| {elements.map(elements => ( | |||
| {elements.map(el => ( | |||
There was a problem hiding this comment.
I was tracking down an error and having a bunch of nested objects using the same variable name was making me slightly bananas. This doesn't strictly need to be changed
jimmoffet
commented
Feb 26, 2024
| ], | ||
| "include": [ | ||
| "./packages/*/src/**/*.json", | ||
| ], |
There was a problem hiding this comment.
Various config changes to permit importing a local .json file in an idiomatic way
jimmoffet
changed the base branch from
main
to
dependabot/npm_and_yarn/babel/traverse-7.23.2
jimmoffet
changed the base branch from
dependabot/npm_and_yarn/babel/traverse-7.23.2
to
main
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a WIP, but don't let me block you from merging it if you're comfortable.
I built into the stub of the fakeCache function Document's suggestFormDetails(). There's a small hard-coded fieldsMap object I was using for testing and a parseJson() function in mock-api.ts that can produce a complete fieldsMap object from a json object (currently being read in from a file).
The primary remaining work is to continue refining the relationship between parsedPDF and the fieldsMap object.
The biggest tension is figuring out groups of elements. The quality of what the parsing routine is able to deliver jumps quite a bit once we're able to accommodate element groups that look like:
For reasons that are hard to explain succinctly, but easy to see in practice, treating all of those objects as independent elements creates lots of friction and extra work for the content author.
The next biggest tension is forcing users to treat every line of non-input text parsed from the pdf as a independent editable object, rather than lumping them together. Google forms has a very opinionated take that privileges architecture simplicity over flexible UX, it's worth looking at for early iterations of our editing UI. We can probably leapfrog a lot of time coming around to the same conclusions.
I tweaked some things in the edit UI mostly for my own education/edification, hopefully those changes will be washed away soon by new incoming designs.
TODO: I barely touched the display side at all. The non-input paragraph element needs an appropriate representation. I noticed the paragraphBlock object in the unwired fieldset comments, but I didn't dig into it. The only change I made was to substitute the label over the input fields with
element.instructionsbecause we have instructions now and it seemed more appropriate thanelement.name.TODO: Make sure the createPrompt() in the paragraph element definition is sensible. I don't totally understand its purpose and implications.
TODO: I'm not sure the paragraph element is appropriately named or actually wants to be tied to a paragraph html tag. Realistically, it's probably going to be hard to avoid a div level container of multiple text tags, unless we get much better at parsing and auto-combining elements. Even then, we'll have to see how much users are willing to put up with in terms of forcing them to edit closely-related chunks of text as separate objects.