gwaft-template-collapsible.php: Added Order Summary as an additional last page.
#1052
Conversation
WalkthroughThe changes introduce new conditional logic in the template processor to handle the "Order Summary" as a separate last page in the collapsible template. The update uses the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Template Processor
participant F as Field Access (rgar)
participant C as Conditional Checker
participant PG as Page Group Handler
T->>F: Retrieve field data using rgar
alt Field is defined
T->>T: Process field normally
else Field is undefined
T->>C: Check if label equals "Order Summary"
alt Label matches "Order Summary"
C->>PG: Add item to page group (-1)
PG->>T: Set page label as "Order Summary"
else
C->>T: Skip field processing
end
end
Suggested Reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
gravity-forms/gwaft-template-collapsible.php (2)
30-35: The Order Summary implementation addresses the PR objective wellThis code block effectively handles the Order Summary as a separate last page, which aligns perfectly with the PR objective. The use of
apply_filters()for the "Order Summary" label provides good flexibility for customization.For even greater robustness, consider using a case-insensitive comparison:
- if ( ! $field && rgar( $item, 'label' ) == apply_filters( 'gwaft_order_summary_label', 'Order Summary' ) ) { + if ( ! $field && strcasecmp(rgar( $item, 'label' ), apply_filters( 'gwaft_order_summary_label', 'Order Summary' )) === 0 ) {
1-106: Consider updating the documentation headerThe code changes are well-implemented, but the documentation header at the top of the file doesn't mention the new Order Summary functionality. Consider adding a brief description of this feature to make it clear to users.
*  * * Instructions: * * 1. Ensure you have All Fields Template installled. * https://gravitywiz.com/gravity-forms-all-fields-template/ * * 2. Install this snippet. No code configuration required. * https://gravitywiz.com/documentation/managing-snippets/#where-do-i-put-snippets * * 3. Enable the collapsible template on any `{all_fields}` merge tag. * `{all_fields:template[collapsible]}` +* +* Features: +* - Displays form fields in collapsible sections based on page breaks +* - Automatically adds Order Summary as a dedicated final section
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gwaft-template-collapsible.php(1 hunks)
🔇 Additional comments (2)
gravity-forms/gwaft-template-collapsible.php (2)
28-29: Good use ofrgar()for safer array accessReplacing direct array access with the
rgar()function is a good improvement as it safely gets values from an array without causing "undefined index" errors if the key doesn't exist.
38-38: Good defensive programming practiceAdding the check for
! $fieldto the hidden fields condition is a good defensive programming practice. This ensures the code doesn't attempt to access properties of undefined fields.
| $field = rgar( $item, 'field' ); | ||
|
|
||
| // Add Order Summary as a separate last page. | ||
| if ( ! $field && rgar( $item, 'label' ) == apply_filters( 'gwaft_order_summary_label', 'Order Summary' ) ) { |
There was a problem hiding this comment.
This check on the label value feels potentially flimsy to me
There was a problem hiding this comment.
This is how the order summary item is:
For regular fields, it is:
So an alternate logic could possibly be "if ( ! field )". But, that could possibly be a false flag is what I wondered.
There was a problem hiding this comment.
The following is a note for myself:
Looking at the code in gw-all-fields-template.php, it looks like $field will always be present unless it's the product summary.
The product summary is added here and the other two instances (where $field is included) are here and here
There was a problem hiding this comment.
That's tricky.
A second option, is to modify the gw-all-fields-template snippet so that the order summary item has an additional key / flag that could be checked here. (Maybe is_order_summary => true?).
Given this is a snippet, I'm okay leaving this as is.
@saifsultanc can you add a comment explaining the conditional / how it's expected to work?
|
|
||
| // Add Order Summary as a separate last page. | ||
| if ( ! $field && rgar( $item, 'label' ) == apply_filters( 'gwaft_order_summary_label', 'Order Summary' ) ) { | ||
| $page_groups[-1][] = $item; |
There was a problem hiding this comment.
Can you explain how the -1 indexing works here?
There was a problem hiding this comment.
I was storing -1 as an arbitary and additional page containing the detail for 'Order Summary'. Do you want to do it as 99129 (or the like)? A big arbitrary number instead.
There was a problem hiding this comment.
Got it!
Would it work to push the order summary item to the end of $page_groups and then set the $page index accordingly?
There was a problem hiding this comment.
I was vary of other potential customizations that may be thrown it by customers, adding additional pages and stuff? Thought to keep it 'separate' from the rest of the stuff. Unless if you suggest that pushing it at the end is a safer way to go forth, I will make the update. Thanks!
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2872221797/79232
Summary
When using the all fields collapsible sections snippet, if there is a pricing field on the form, the order summary is included in its own section. However, this also causes the page names to be showing incorrectly on the sections, and the section the order summary is included in does not have a title.
Matt A's loom explaining the issue:
https://www.loom.com/share/48c97b4b77054d36ab5c64973bd8ea84
Handle the "Order Summary" page has been proposed with this PR.
How this update works:
https://www.loom.com/share/728fc541438d4041a8e8e91ceee87e58