When you add new line items to an order, there's no visual indication that the entire form must be saved for the line items to be saved via the line item reference field. This might not be a big deal, since we might want to convert away from using that widget entirely... but in the meantime, we should consider adding a blocks / fields UI style warning indicating the form needs to be submitted to preserve changes.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 1059400.patch | 763 bytes | artusamak |
Comments
Comment #1
pcambraTagging
Comment #2
artusamakHere a small patch doing the trick.
Comment #3
recidive commentedLooks good:
https://skitch.com/recidive/r1cux/order-3-drupal-commerce
It can be picked from this commit:
https://github.com/recidive/drupalcommerce/commit/c4898f616823e491195814...
Comment #4
rszrama commentedCurrently this patch shows the warning as soon as the "Add line item" button is clicked showing which type of line item to add to the order. However, we only need to show it when a line item is actually added to the order - i.e. the "Add product" button is clicked. We can't rely on that, though, because there might be different buttons for different types of line items...
I think perhaps what should happen is when a line item is actually saved, we should toggle a boolean in the $form_state indicating the message should be displayed. This will be preserved regardless of what other buttons are clicked. This code should go somewhere near the instance of commerce_line_item_new() around line 1100. Then the original patch should be updated to use this boolean... perhaps just name it $form_state['line_item_save_warning'].
Comment #5
pcambraRepo: git.drupal.org:sandbox/pcambra/1081200.git
Branch: 1059400
Diff: http://drupalcode.org/sandbox/pcambra/1081200.git/commitdiff/0533e39ea60...
Comment #6
recidive commentedIsn't it better to use isset() rather than !empty() in this case? I guess it will not have any practical issues but the following code looks more appropriate IMO:
Comment #7
rszrama commentedGiven the usage in Drupal as a whole, it's probably enough to just have the single condition being either
if (!empty($form_state['line_item_save_warning']))See for example the various uses of is_new in the core modules. It'll pass the PHP notice if the key doesn't exist, and it'll ensure it's not FALSE. I'll flatten the statement to this and commit.
Thanks for the work / review on this guys. : )