Add collapsible feature in forms

[CarolineDenis]

Fixes #1995

[CarolineDenis]

part1.mov

[CarolineDenis]

part2.mov

[maxpatiiuk]

Can you please tell why useBlockerHandler didn't work for you? If there is a bug with useBlockerHandler then it would be important to know about it and fix it.

If it's just a matter of having to add parentResource prop, then surely it's simpler than creating a context and two React.useEffects with setTimeouts?

Please tell if there are issues with the following:

useBlockerHandler(parentResource, relationship, hangleExpand);

Simply add this prop to FormTable:

readonly parentResource: SpecifyResource<AnySchema> | undefined;

And in IntegratedRecordSelector, just add this to FormTableCollection:
parentResource={collection.related}

(and no need even to change anything in FormTableCollection)

[CarolineDenis]

@maxpatiiuk I used in this latest version "useAllSaveBlockers", it makes the code way more easy to read and I could get rid of the context and avoid a few line of code 👍
I still had to use a useEffect and time out. One to avoid to many renderers, the other because it would uncollapse the first time but wouldn't if you add a record with a mistake than wait than collapse.
The useBlockerHandler didn't work because when the element is collapsed it's hidden and therefore non-focusable.

ps: the behavior is the same than on the video with the new code, if there is an error in one of the subform it will uncollapse.

[grantfitzsimmons]

I believe the Specify 6 symbols are more intuitive, where there is a right-facing arrow when collapsed and a downward facing arrow when expanded.

I don't know why I feel this way, since up to collapse and down to expand seems more "logical"– it just feels inverted somehow.

[maxpatiiuk]

@CarolineDenis the useBlockerHandler hook is intended to automatically uncollapse the subview if the user clicks on the "Save" button, rather than prevent the user from collapsing the subview if there is an error (based on a 2 second timer - a cludge).

I was thinking uncollapsing on save button click rather than timer would be less confusing to the user, but am open to suggestions.

[maxpatiiuk]

For example consider a case where a field in a collapsed subview becomes invalid as part of an edit to a field outside the subview. That could happen because of business rules. That could also happen because (on xml-editor branch) users can embed the fields from a dependent -to-one relationship directly on the main form, without using a sub view.

[maxpatiiuk]

And even if you want to go with the current approach, I still don't think a timer is the right approach.

Why not just disable the collapse button if there is an error OR even better, when user clicks on the button, focus the error field instead of collapsing? (that can be done by calling form.reportValidity(). You can add readonly form: HTMLFormElement | undefined; to FormContext->FormMetaType to accomplish this). That, while being a bit more complicated, results in a better UX.

Just look at how few usages of setTimeout there are in the codebase - and all of them are there for a good reason (i.e, for performance, or for auto-dismissing messages)

[CarolineDenis]

@maxpatiiuk,
if disabeling the collapse button with "hasBlockers": when adding a new prep, the type of prep is not selected yet so the button is disabled as it should but when we choose the prep hasBlockers becomes false but since there is no re-render the button stays disabled until reload

if using useBlockerHandler: there is an error because the form is not focusable see screen shots.

[maxpatiiuk]

I had this error even with the setTimeout hack:

[maxpatiiuk]

Feature request: either hide the add/remove buttons when view is collapsed, OR better: expand the subview when add/remove/slider is pressed.

[maxpatiiuk]

... but since there is no re-render the button stays disabled until reload

that seems like a bug (either with the hook or with your code). That shouldn't be happening as useAllSaveBlockers is supposed to trigger re-render on blockers change.

[maxpatiiuk]

I fixed all of the remaining issues and fixed the suggested changed and opened a separate PR - #3680

My fix didn't use setTimeout and didn't use useBlockerHandler.

I opened a separate PR so that you have a choice of fixing all the issues on your own on this branch and only afterward looking at that PR for reference/suggestions, OR continuing with that PR instead of this one.

If you want to carry over just that single commit from that branch to this, you could type git cherry-pick 1417a8c

In either case, the code should still undergo testing as I did only basic testing

[maxpatiiuk]

Looks good
Hope the code makes sense - it's pretty much what you had, just a bit cleaned up

[CarolineDenis]

Looks good
Hope the code makes sense - it's pretty much what you had, just a bit cleaned up

yes makes a lot of sense. Thank you for the push

[grantfitzsimmons]

The collapse behavior is inverted for the Collecting Event subview.

		<row>
			<cell type="subview" viewname="CollectingEventSub" id="21" label="Collecting Event" name="collectingEvent" colspan="13" rows="5"/>
		</row>

https://herbrbge71423-issue-1995.test.specifysystems.org/specify/view/collectionobject/349648/?recordsetid=178

Screen.Recording.2023-08-24.at.7.24.01.PM.mov

It is collapsed by default, though in the form definition the collapse attribute (which would be inside of the initialize attribute) is not present.

[grantfitzsimmons]

I was so wrong... it was working correctly, the form definition tripped me up.

Works well with the initialize="collapse=true" and initialize="collapse=false" attributes in the XML as well.