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

gwaft-template-collapsible.php: Added Order Summary as an additional last page. #1052

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions gravity-forms/gwaft-template-collapsible.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,18 @@
$pages = $data['form']['pagination']['pages'];
$page_groups = array();
foreach ( $data['items'] as $item ) {
$field = $item['field'];
$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' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check on the label value feels potentially flimsy to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how the order summary item is:
Screenshot 2025-03-29 at 4 06 19 PM

For regular fields, it is:
Screenshot 2025-03-29 at 4 06 41 PM

So an alternate logic could possibly be "if ( ! field )". But, that could possibly be a false flag is what I wondered.

Copy link
Contributor

@veryspry veryspry Mar 31, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

// Storing order summary page as '-1' to be the last page, and avoid conflicts with the actual page numbers.
$page_groups[-1][] = $item;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how the -1 indexing works here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it!

Would it work to push the order summary item to the end of $page_groups and then set the $page index accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

Copy link
Contributor

Choose a reason for hiding this comment

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

I do think throwing it on the end is a safer bet, but that said, I'm actually not super familiar with other potential customizations etc. I'll leave the final call to you!

If you do go with -1, could you add a note explaining the logic?

$pages[-1] = apply_filters( 'gwaft_order_summary_label', 'Order Summary' );
continue;
}

// Skip hidden fields.
if ( $field->type === 'hidden' || $field->visibility === 'hidden' ) {
if ( ! $field || $field->type === 'hidden' || $field->visibility === 'hidden' ) {
continue;
}
// Adjust pageNumber to be zero-based.
Expand Down
Loading