Inline errors landed in core recently. When combined with the price field, the result is not good, because the currency selector is pushed to the next row (instead of the error itself being below both the amount/currency and description rows).

We need to see if we can move the placement of the error (or tweak the css to achieve what we need), or move the error to the parent form (making sure it doesn't go more than 1 level above, to account for Inline Entity Form).

CommentFileSizeAuthor
#7 commerce_price-fix-inline-form-errors-on-price-element-2509702-7.patch1.75 KBjsacksick
#7 price-inline-validation-after-patch.png19.09 KBjsacksick
#5 inline-form-errors-product.png129.43 KBjsacksick
price-inline-error.png22.4 KBbojanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Priority: Normal » Minor

Inline errors were reverted and moved to an optional module.

pramod_patil’s picture

pramod_patil CreditAttribution: pramod_patil at Faichi Solutions Pvt Ltd commented
Assigned: Unassigned » pramod_patil
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Assigned: pramod_patil » Unassigned
Status: Active » Postponed

It is an experimental module now.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Postponed » Active

No longer experimental, needs to be retested.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Issue summary: View changes
FileSize
inline-form-errors-product.png129.43 KB

Inline errors look terrible on all our fields that have a selector (the selector is pushed to the next row), see screenshot below:

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented

It is possible to disable inline errors for some elements by adding the following flag to a form element: '#error_no_message' => TRUE
but we should of course better find a way to fix it :).

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Status: Active » Needs review
FileSize
price-inline-validation-after-patch.png19.09 KB
commerce_price-fix-inline-form-errors-on-price-element-2509702-7.patch1.75 KB

So, I found a solution to fix this by setting the #error_no_message flag on the number element, and by adding a element validate callback on the price element itself...

After patch:

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

We should see if this can be abstracted into https://github.com/drupalcommerce/commerce/blob/8.x-2.x/src/Element/Comm... as some kind of helper.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Assigned: Unassigned » bojanz
Priority: Minor » Normal
Status: Needs review » Needs work

I told Jonathan to hold off with a helper until we have an actual second use case. Smaller risk of misabstraction that way.

I got up to speed with IFE again, and it has made little progress since the 8.0.x days. I've confirmed that Jonathan's fix is the only sensible one.
I did want two changes:
1) Rename validatePrice() to moveInlineErrors() to make it obvious what's happening
2) Perform the move only when IFE is enabled. That way the non-IFE use case doesn't regress.

  • bojanz committed aafd17c on 8.x-2.x authored by jsacksick 
    Issue #2509702 by jsacksick, bojanz: Inline errors look terrible on the...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs work » Fixed

There we go. Thanks, Jonathan.

Status: Fixed » Closed (fixed)

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