Currently, the review pane shows an edit link for every pane, no matter if its visible or not. This can lead to strange behaviour, when a checkout step only consists of invisible panes and therefore is skipped during the checkout process anyway.

CommentFileSizeAuthor
checkout_review_hide_edit_link_for_invisible_panes.patch1.29 KBagoradesign
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agoradesign created an issue. See original summary.

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Title: Hide edit links for invisible checkout panes on checkout review » Review pane shows summaries of non-visible panes
Category: Feature request » Bug report
Priority: Minor » Normal

Agreed. We need to skip the entire foreach for panes that aren't visible.

  • bojanz committed 5fbbf6f on 8.x-2.x authored by agoradesign 
    Issue #2715269 by agoradesign, bojanz: Review pane shows summaries of...
bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Needs review » Fixed

Done.

agoradesign argued that certain hidden panes should have a chance to show a review.
That would be an unplanned feature that I haven't seen on other sites, so let's leave that to a followup.

agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented
Parent issue: » #2710973: [meta] Finish the checkout functionality

added the parent issue

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Fixed » Needs work

Merging #2715311: Introduce isPaneSummaryVisible() to allow checkout panes hide its pane, but show its summary into this issue, since this is where the discussion started.

After rediscussing with agoradesign, rszrama and mglaman, we agreed that the pane summaries should by default be shown even when the pane is hidden. The contact information pane is a good example of this.

We definitely want to hide the Edit link if $pane->isVisible() is FALSE, like the original patch did.
What remains is deciding whether summary visibility should be controlled via a new method (isPaneSummaryVisible()) or the return value of buildSummary().

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Title: Review pane shows summaries of non-visible panes » Define the behavior of non-visible panes on Review
agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented

I've also thought about the visibility thing in the meantime, after I've proposed the patch in #2715311: Introduce isPaneSummaryVisible() to allow checkout panes hide its pane, but show its summary.

I believe:

  1. The new function isPaneSummaryVisible() is still a good idea imho. It allows great flexiblity for all panes + it gives the check we're doing a nice and understandable name
  2. In contrary to the current implementation and my patch, the review pane should not additionally check for empty summary. Instead this should all be handled by isPaneSummaryVisible() instead.
  3. As a consequence, isPaneSummaryVisible() should not default to TRUE, but default to check for an empty summary

  • bojanz committed da10bf2 on 8.x-2.x authored by agoradesign 
    Issue #2715269 by agoradesign, bojanz: Define the behavior of non-...
bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Needs work » Fixed

I consulted with mglaman and rszrama and we concluded that the implicit approach is simpler.

So I've basically committed #1 + a comment on the buildPaneSummary method.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.