Problem/Motivation

The form to create bundles would gain a lot in UX if these 2 points were avoided/resolved:

1- Users currently have to create a bundle, then create a field and then come back to the bundle form to set the source field.
2- Users currently have to select the option "gather exif" data, save the form, and then reload the bundle edit form to have the actual exif mapping fields available.

I understand that the point #1 is being solved in #2625854: Provide default source_field when creating new media entity bundles, so let's leave it out of this issue.

For the point in #2, the ideal scenario would be to rebuild the form container by ajax with the exif fields as soon as the user selects this option in the dropdown.

Proposed resolution

Option A: Update the form using ajax to present all fields available to mapping without leaving the page.

Option B: Improve the description of the "Whether to gather exif data" option, indicating that the user needs to save and reload the page in order to have these fields available.

Remaining tasks

- Decide which solution is best
- Implement it

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#16 interdiff.txt1.24 KBmarcoscano
#16 improve-description-of-exif-option-2757835-16.patch499 bytesmarcoscano
#11 improve-description-of-exif-option-2757835-11.patch1.51 KBshreya shetty
#5 improve-description-of-exif-option-2757835-5.patch1.64 KBshreya shetty
#2 improve-description-of-exif-option-2757835-2.patch863 bytesmarcoscano

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Assigned: marcoscano » Unassigned
Status: Active » Needs review
Issue tags: +D8Media
StatusFileSize
new improve-description-of-exif-option-2757835-2.patch863 bytes

Patch attached if we decide to go for option B. The text being changed is:

t('Gather exif data using exif_read_data().')

into

t('Gather exif data using exif_read_data(). After choosing this option, you need to save and reload this page in order to have the additional fields available on the mapping section below.')

slashrsm’s picture

slashrsm commented
Status: Needs review » Needs work
Issue tags: +Novice

Could we rather reload the form through AJAX when the value changes?

Another improvement came to my mind when I was looking at this patch. We disable the dropdown if exif_read_data() is missing. I think that it would be nice to add an explanation message in this case.

shreya shetty’s picture

shreya shetty commented
Assigned: Unassigned » shreya shetty
shreya shetty’s picture

shreya shetty commented
Status: Needs work » Needs review
StatusFileSize
new improve-description-of-exif-option-2757835-5.patch1.64 KB

I have provided the functionality which is mentioned in this issue . Please review the patch

Status: Needs review » Needs work

The last submitted patch, 5: improve-description-of-exif-option-2757835-5.patch, failed testing.

The last submitted patch, 5: improve-description-of-exif-option-2757835-5.patch, failed testing.

shreya shetty’s picture

shreya shetty commented
Status: Needs work » Needs review
shreya shetty’s picture

shreya shetty commented
Assigned: shreya shetty » Unassigned
slashrsm’s picture

slashrsm commented
Status: Needs review » Needs work

General approach of the patch looks good. There are few coding/documentation standards that need to be fixed. Running CodeSniffer on it would help.

Based on the nature of the test fails I suspect that the upload field on the entity form is missing or not not being loaded correctly. I didn't run then so I might be wrong though.

shreya shetty’s picture

shreya shetty commented
Status: Needs work » Needs review
StatusFileSize
new improve-description-of-exif-option-2757835-11.patch1.51 KB

Thank You for the review . I think we need to update the test for this . And fixed all the documentation error pointed all by code sniffer for this particular file.

Status: Needs review » Needs work

The last submitted patch, 11: improve-description-of-exif-option-2757835-11.patch, failed testing.

The last submitted patch, 11: improve-description-of-exif-option-2757835-11.patch, failed testing.

shreya shetty’s picture

shreya shetty commented
Status: Needs work » Needs review
slashrsm’s picture

slashrsm commented
Status: Needs review » Needs work
marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Status: Needs work » Needs review
StatusFileSize
new improve-description-of-exif-option-2757835-16.patch499 bytes
new interdiff.txt1.24 KB

Don't know why the testbot is failing, I have green in local so it is difficult to find the problem...

In any case:

+++ b/src/Plugin/MediaEntity/Type/Image.php
@@ -186,6 +200,9 @@ class Image extends MediaTypeBase {
+      '#ajax' => [
+        'callback' => '::ajaxTypeProviderData',
...
       '#disabled' => (function_exists('exif_read_data')) ? FALSE : TRUE,

If we are reusing the ajaxTypeProviderData, there is no need to redeclare it inside the plugin, isn't it?

The patch attached works as well, without duplicating it inside the image plugin. (Actually the added ajaxTypeProviderData there wasn't being called at all)

secretsayan’s picture

secretsayan commented

Yes exactly @marcoscano. The function ajaxTypeProviderData wasn't being called at all. I was also a bit confused about the test bot. Anyways , I can confirm this patch is working.

  • slashrsm committed af76ed6 on 8.x-1.x authored by marcoscano 
    Issue #2757835 by marcoscano, Shreya Shetty, secretsayan: Load possible...
slashrsm’s picture

slashrsm commented
Status: Needs review » Fixed

Committed. Thanks!

Status: Fixed » Closed (fixed)

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