Skip to content

Conversation

@barthc
Copy link
Contributor

@barthc barthc commented Oct 15, 2024

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2721901699/72085

Summary

The gpnf-disable-sessions snippet prevents parameter-based dynamic population from functioning in the child form. The request vars are stored in the session and then used to populate the nested form field. Since this snippet disables sessions, the dynamic population won't work, hence the issue.

The fix here is to store the request vars in a cookie and use it to populate the nested form fields.

@github-actions
Copy link

github-actions bot commented Oct 15, 2024

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against 1b369f2

@barthc barthc force-pushed the barth/fix/72085-populating-values branch from 222b8f6 to 1b369f2 Compare October 15, 2024 11:50
@spivurno spivurno self-requested a review October 15, 2024 16:38
Copy link
Contributor

@spivurno spivurno left a comment

Choose a reason for hiding this comment

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

@barthc This snippet is intended to be a solution for the too-large cookie issue we began solving here:

https://github.com/gravitywiz/gp-nested-forms/pull/276/

I'm hesitant to reintroduce a cookie here. 😅

I think the play might be to continue the work of simplifying the cookie by moving the query parameters from the cookie and pass them to the AJAX requests as we did with the field values.

https://github.com/gravitywiz/gp-nested-forms/pull/276/files#diff-39903e76cb5b16955fb5a0619493700912ebd6255c73220522f358a79de23577R2262

@barthc barthc closed this Oct 16, 2024
@barthc barthc deleted the barth/fix/72085-populating-values branch October 16, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants