IBX-6856: Added mimeTypes field to ImageFormMapper #1021

ciastektk · 2023-12-01T10:49:55Z

Question	Answer
Tickets	https://issues.ibexa.co/browse/IBX-6856
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	yes
License	GPL-2.0

ibexa/core#300
ibexa/content-forms#55
https://github.com/ibexa/image-editor/pull/80

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

webhdx · 2023-12-04T10:37:47Z

src/lib/FieldType/Mapper/ImageFormMapper.php

+    {
+        $mimeTypeChoiceList = [];
+        foreach ($this->allowedMimeTypes as $mimeType) {
+            $extensions = implode(', ', $this->mimeTypes->getExtensions($mimeType));


In my opinion we should display extensions in the following format *.png, *.jpg to indicate the list contains actual file extensions. Also it would help to avoid displaying labels like png (png), instead you'll get png (*.png).

I like this, but I will ask Ida for confirmation before I provide changes.

Approved by Design Team
697a859

src/lib/FieldType/Mapper/ImageFormMapper.php

src/bundle/Resources/public/js/scripts/fieldType/ezimage.js

mnocon

Tested all the 4 PRs together.

It works well for the Image fieldtype, but there are two issues with the ImageAsset fieldtype:

Issue 1

If a non-allowed image type is uploaded to the system using the ImageAsset fieldtype then the upload fails (that's expected) but the error is not handled in a nice way

Steps:

Create a CT with an imageasset fieldtype - set the limitation that only GIFs are allowed
Start creating a Content Item of that type
Select "Browse" and upload a JPG file

In other cases - for example when the image size is too big - the error is handled as it should be.

Issue 2

Entering Version comparison for an Image Asset fieldtype results in an error.

Steps:

Create a Content Type with image asset fieldtype
Create a Content Item of that Content Type
Go to Versions -> Enter version comparison
Switch to Single view

mnocon · 2023-12-07T12:52:33Z

Also a suggestion (maybe we should consult the UX team how it should be done)

Can we display the allowed mime types like we do for filesize?

Currently it looks like this - there's no way of telling that only JPGs are supported, you have to publish the image to find out what's allowed.

ciastektk · 2023-12-08T07:55:11Z

Also a suggestion (maybe we should consult the UX team how it should be done)

Can we display the allowed mime types like we do for filesize?

Currently it looks like this - there's no way of telling that only JPGs are supported, you have to publish the image to find out what's allowed.

I added section with allowed mime types:

ibexa/content-forms@e08900b
d9ccf3e

Fix for Issue 2: ibexa/core@93eaa4a

mnocon · 2023-12-08T13:48:51Z

Thanks! Issue 2 is gone 💪

About the suggestion for user - it looks good now, just one, tiny nitpick: can we add a space between the various types so it's easier to read?

ciastektk · 2023-12-08T14:05:30Z

Thanks! Issue 2 is gone 💪

About the suggestion for user - it looks good now, just one, tiny nitpick: can we add a space between the various types so it's easier to read?

Fixed: 30417e4

lserwatka · 2023-12-08T14:15:26Z

I don't like this design to be frank. @NataliaBecla can we do better here? Mime Types is a technical language. Why not just come up with a mapping and display to end user something like ".png, .jpg, .jpeg, .gif" etc. This is more understandable. @SylvainGuittard @adamwojs

webhdx · 2023-12-11T13:38:22Z

@lserwatka This is the same conclusion we came up with when @ciastektk added mime type configuration in field settings. The list also shows possible file extensions on top of mime types.

konradoboza · 2023-12-12T12:36:10Z

Added JS validation to the file input, so issue 1 should be gone now. Also, mime-types were replaced by the extensions when displaying notice. Updated corresponding content-forms PR as well, please check ibexa/content-forms@841fabf.

mnocon · 2023-12-14T13:14:34Z

Issue 1 is now gone 🎉

New things that I've found:

1. For Image fieldtype the validation is now displayed twice

2. The translations for alternative text in image asset is missing

This one is also happening on Nightly

3. The displayed message is not consistent with the design

Design: https://www.figma.com/file/4sbNQyNLhFFwxWDH89Rf9X/%F0%9F%9F%A2-Ibexa-Design-System?type=design&node-id=3329%3A445543&mode=design&t=ltb1PxLccYC7CM63-1

Design:

Actual:

(also the look of other states is not consistent with the Figma design)

mnocon

I will report these issues as follow-ups, so that we can tackle them in the future - the core mechanics are in place now 👍

sonarcloud · 2023-12-14T13:35:21Z

Quality Gate failed

Failed conditions

14.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

github-actions bot assigned dew326, ciastektk, mikadamczyk, Nattfarinn and ViniTou Dec 1, 2023

ciastektk force-pushed the ibx-6856-added-mime-types-field-to-image-form-mapper branch 2 times, most recently from 9272564 to b114a01 Compare December 1, 2023 11:20

ciastektk mentioned this pull request Dec 1, 2023

IBX-6856: Added mime types limitation for ezimage field type ibexa/core#300

Merged

6 tasks

ciastektk requested a review from a team December 1, 2023 11:44

ciastektk added Feature New feature request Ready for review labels Dec 1, 2023

ciastektk mentioned this pull request Dec 1, 2023

IBX-6856: Added view variable mime types for ezimage field type ibexa/content-forms#55

Merged

6 tasks

ciastektk requested a review from a team December 1, 2023 11:49

webhdx reviewed Dec 4, 2023

View reviewed changes

webhdx requested a review from a team December 4, 2023 10:38

ciastektk requested a review from webhdx December 4, 2023 12:04

Nattfarinn approved these changes Dec 4, 2023

View reviewed changes

konradoboza reviewed Dec 4, 2023

View reviewed changes

src/lib/FieldType/Mapper/ImageFormMapper.php Show resolved Hide resolved

konradoboza approved these changes Dec 5, 2023

View reviewed changes

konradoboza requested a review from a team December 5, 2023 07:53

dew326 approved these changes Dec 5, 2023

View reviewed changes

src/bundle/Resources/public/js/scripts/fieldType/ezimage.js Show resolved Hide resolved

src/bundle/Resources/public/js/scripts/fieldType/ezimage.js Show resolved Hide resolved

ciastektk added Ready for QA and removed Ready for review labels Dec 6, 2023

Base automatically changed from ibx-6937-used-number-type-field-to-set-image-max-size to main December 6, 2023 08:00

ciastektk force-pushed the ibx-6856-added-mime-types-field-to-image-form-mapper branch from bfa6283 to 9efe22c Compare December 6, 2023 08:45

mnocon requested changes Dec 7, 2023

View reviewed changes

konradoboza requested review from dew326, mnocon and a team December 12, 2023 12:34

konradoboza force-pushed the ibx-6856-added-mime-types-field-to-image-form-mapper branch 2 times, most recently from a4b9cb7 to e38ca94 Compare December 12, 2023 12:56

dew326 approved these changes Dec 12, 2023

View reviewed changes

mnocon approved these changes Dec 14, 2023

View reviewed changes

ciastektk and others added 9 commits December 14, 2023 14:26

Added mimeTypes field to ImageFormMapper

5d15145

Added data attribute with allowed-file-types

b45e949

[PHPStan] Regenerated baseline

e007653

Extracted translations

4428d14

Changed image extension format

2447934

Changed mimeTypes form field as optional

312ec2e

Added allowed mime types info to drop zone

1586f60

Added space between allowed mime types

4682146

added validation of image extensions

098d250

ciastektk force-pushed the ibx-6856-added-mime-types-field-to-image-form-mapper branch from e38ca94 to 098d250 Compare December 14, 2023 13:26

Removed ignored error entry

5c1af65

ciastektk merged commit 719f2cf into main Dec 14, 2023
15 of 23 checks passed

ciastektk deleted the ibx-6856-added-mime-types-field-to-image-form-mapper branch December 14, 2023 13:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBX-6856: Added mimeTypes field to ImageFormMapper #1021

IBX-6856: Added mimeTypes field to ImageFormMapper #1021

ciastektk commented Dec 1, 2023 •

edited

Loading

webhdx Dec 4, 2023

ciastektk Dec 4, 2023

ciastektk Dec 4, 2023 •

edited

Loading

mnocon left a comment

mnocon commented Dec 7, 2023

ciastektk commented Dec 8, 2023 •

edited

Loading

mnocon commented Dec 8, 2023

ciastektk commented Dec 8, 2023

lserwatka commented Dec 8, 2023 •

edited

Loading

webhdx commented Dec 11, 2023

konradoboza commented Dec 12, 2023 •

edited

Loading

mnocon commented Dec 14, 2023

mnocon left a comment

sonarcloud bot commented Dec 14, 2023

IBX-6856: Added mimeTypes field to ImageFormMapper #1021

IBX-6856: Added mimeTypes field to ImageFormMapper #1021

Conversation

ciastektk commented Dec 1, 2023 • edited Loading

Checklist:

webhdx Dec 4, 2023

Choose a reason for hiding this comment

ciastektk Dec 4, 2023

Choose a reason for hiding this comment

ciastektk Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

mnocon left a comment

Choose a reason for hiding this comment

Issue 1

Issue 2

mnocon commented Dec 7, 2023

ciastektk commented Dec 8, 2023 • edited Loading

mnocon commented Dec 8, 2023

ciastektk commented Dec 8, 2023

lserwatka commented Dec 8, 2023 • edited Loading

webhdx commented Dec 11, 2023

konradoboza commented Dec 12, 2023 • edited Loading

mnocon commented Dec 14, 2023

Issue 1 is now gone 🎉

1. For Image fieldtype the validation is now displayed twice

2. The translations for alternative text in image asset is missing

3. The displayed message is not consistent with the design

mnocon left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 14, 2023

Quality Gate failed

ciastektk commented Dec 1, 2023 •

edited

Loading

ciastektk Dec 4, 2023 •

edited

Loading

ciastektk commented Dec 8, 2023 •

edited

Loading

lserwatka commented Dec 8, 2023 •

edited

Loading

konradoboza commented Dec 12, 2023 •

edited

Loading