Display 'default_step' property of @CommerceCheckoutPane plugins in administration

Created on 28 August 2024, 21 days ago
Updated 12 September 2024, 7 days ago

Problem/Motivation

@CommerceCheckoutPane plugins have a 'default_step' property, see https://docs.drupalcommerce.org/v2/developer-guide/checkout/checkout/

/**
 * Provides a custom message pane.
 *
 * @CommerceCheckoutPane(
 *   id = "my_checkout_pane_custom_message",
 *   label = @Translation("Custom message"),
 *   display_label = @Translation("Another display label"),
 *   default_step = "_sidebar",
 *   wrapper_element = "fieldset",
 * )
 */

In many cases this default step has technical implications and should not be changed or may result in weird behavior. Restoring is not easy then and you don't know where the pane originally belonged without looking into code or sth. like that.

Steps to reproduce

Move a checkout pane into a region where it makes no sense, but it's not obvious.

Proposed resolution

For that reason I'd suggest do print the default_step beside the label, for example <small>Default step: order_information</small> or something like that.

Future considerations:

  • Maybe a future step could then be to add an "allowed_steps" property which takes an array of all steps where the pane makes sense, validated on safe. But this is already an important first step for improving the UX / SX with just a little change.
  • Another future step might be to add a "Reset" button, if it was moved out of the default step
  • Allow admin_description for @CommerceCheckoutPane plugins RTBC is also related as it could provide more details.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Feature request
Status

Needs review

Version

3.0

Component

Checkout

Created by

🇩🇪Germany Anybody Porta Westfalica

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Anybody
  • 🇩🇪Germany Anybody Porta Westfalica

    Before we implement this, what do the maintainers say?

  • 🇩🇪Germany Anybody Porta Westfalica
  • 🇩🇪Germany Anybody Porta Westfalica

    @jacksicksack: Further thoguhts or details where and how this should be placed?

    I ran into this in each commerce project where we needed to place the checkout panes in different regions.

  • 🇩🇪Germany Anybody Porta Westfalica
  • 🇩🇪Germany Grevil

    Just had a quick chat on slack with @jsacksick if we should implement this feature. His answer:

    Why not [...]

    He also said:

    But I'd consider this low prio TBH

    I think this is particularly important for payment panes

    But not sure how you can clarify this from the UI

    I'd suggest we create a first MR and wait for feedback 👍

  • 🇩🇪Germany LRWebks Porta Westfalica
  • Assigned to LRWebks
  • Status changed to Needs work 8 days ago
  • 🇩🇪Germany LRWebks Porta Westfalica
  • Status changed to Needs review 8 days ago
  • 🇩🇪Germany LRWebks Porta Westfalica
  • Pipeline finished with Failed
    8 days ago
    Total: 711s
    #280089
  • 🇩🇪Germany LRWebks Porta Westfalica

    Now, the same PHPUnit error as in Allow admin_description for @CommerceCheckoutPane plugins RTBC occurs, which again seems to be unrelated to the changed made in the MR.

  • Status changed to Needs work 7 days ago
  • 🇩🇪Germany Anybody Porta Westfalica

    @lrwebks this is simply the wrong approach taken. Please use the same approach for output like you already did well in 🐛 Mapped media fields are overridden with metadata on translation save Needs review !
    So I'm a bit disappointed why you hacked it this way here now...

    Having a dedicated class then like there, (e.g. pane-default-step) we can then discuss how to style it less prominent. For the first step please make the font size small and the text in italics. Label and value should have dedicated classes like
    <div class="pane-default-step"><span class="pane-default-step__label">Default region:</span> <span class="pane-default-step__value">main</span></div>

    After wards please remove the Draft state from the MR.

  • Status changed to Needs review 7 days ago
  • 🇩🇪Germany LRWebks Porta Westfalica

    I have now implemented the system in the same way as Allow admin_description for @CommerceCheckoutPane plugins RTBC as well as added CSS and a proper test. I agree, that my previous approach was definitely flawed, yet I am still convinced that the use of the t() function is necessary here, as we need a way to translate the "Default Step:" label, and concatenating other strings with a t() function that only wraps around the label violates Drupal standards and undermines the purpose of t(), see t() Concatenation Dos and Don'ts

  • Pipeline finished with Failed
    7 days ago
    Total: 491s
    #281018
Production build 0.71.5 2024