Display 'default_step' property of @CommerceCheckoutPane plugins in administration

Created on 28 August 2024, 4 months ago
Updated 19 September 2024, 3 months 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 work

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 3 months ago
  • 🇩🇪Germany lrwebks Porta Westfalica
  • Status changed to Needs review 3 months ago
  • 🇩🇪Germany lrwebks Porta Westfalica
  • Pipeline finished with Failed
    3 months 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 3 months 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 3 months 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
    3 months ago
    Total: 491s
    #281018
  • Status changed to Needs work 3 months ago
  • 🇩🇪Germany Grevil

    As there are a few unresolved threads, which do not seem to require maintainer feedback, I'll set the issue to "Needs Work" again.

  • 🇩🇪Germany Anybody Porta Westfalica

    @lrwebks you can proceed, I left comments.

  • Pipeline finished with Failed
    3 months ago
    Total: 775s
    #292064
  • Pipeline finished with Failed
    3 months ago
    Total: 675s
    #292123
  • Pipeline finished with Success
    3 months ago
    Total: 575s
    #292190
  • 🇩🇪Germany lrwebks Porta Westfalica
  • 🇩🇪Germany lrwebks Porta Westfalica

    Hier ein Screenshot des aktuellen Standes:

  • 🇩🇪Germany Anybody Porta Westfalica

    Final comment, then I think this is fine for maintainer feedback!

    Resulting screenshot looks nice, I like it and already used the information in a project. Small but helpful for site-builders!!

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

    Nice, ready for maintainer feedback or merge :)

  • Pipeline finished with Success
    3 months ago
    Total: 1432s
    #292335
Production build 0.71.5 2024