Closed (fixed)
Project:
Commerce Core
Version:
3.0.x-dev
Component:
User experience
Priority:
Normal
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
29 Aug 2024 at 12:44 UTC
Updated:
30 Oct 2024 at 14:35 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
jsacksick commentedSo... the way we handle the Details (closed) option is kinda hackish, but not sure how to make this cleaner...
Additionally, I'm wondering whether the display label we expose should only "Override" if filled? Basically, whether we should default to the plugin definition display label, or just output an empty text field?
Comment #4
jsacksick commentedThe opened MR is really just a first pass and seems to work :). Making this a "Release blocker" just to keep this issue visible, but this is more like a nice to have IMHO.
Comment #5
jsacksick commentedCheckout settings UI:
The billing information pane with an updated wrapper element (details):
Comment #6
jsacksick commentedPut the display label override behind a checkbox "Override the display label".
Also, created a custom form element thus removing the need of mapping the wrapper element... called it "commerce_details_open", since the default details element is not opened by default.
Comment #7
anybodyWhaooo thank you, this is really nice! :)
I'll review this one. And or course I'm absolutely fine with superseding #3420415: Add option to make coupon code checkout pane collapsible (details) then! Impressive work and nice to also have the label overridable now!
Comment #8
anybodyYeah really nice one, just found one thing so far. BTW super nice "empty checkbox form states api"-combination pattern! Like it!
Comment #9
jsacksick commentedMain thing I'm wondering right now is whether I should skip the non standard/default element (Details), which is basically the core "details" element, but with the #open attribute defaulting to TRUE, using a custom element defined by Commerce "commerce_details_open".
Perhaps it is better to stick to the default elements and not introduce the custom one... thoughts?
Comment #10
anybodyWell on the one hand, I think we need it, otherwise we'd need a further property for the
#open => TRUEflag?On the other hand you could just use
containerif it should be open. But maybe that's enough (at least for now), I can't imagine a case where you really need a closable details element that's open by default...I'm fine with both decisions and think it's an edge-case at least.
So YAGNI?
Comment #11
jsacksick commentedSo let's ditch the custom form element then... We have fieldset or container for that...
I haven't really tested translability either, don't know if we need additional work on that front...
Comment #14
jsacksick commentedDecided to merge the MR, so it can be part of the Commerce 3.0.0-alpha1 release and we can get feedback before a full Commerce 3.0.0 release.
Comment #15
anybodyWHAO! Thank you SO much! You're genius :)
I closed the other issue.