Skip to content

Conversation

@tufahu
Copy link
Contributor

@tufahu tufahu commented Oct 3, 2018

Description

With this fix, developers can add custom catalog image types like SVG from di.xml configuration.
In Catalog ImageUploader allowed extensions can be customized, but allowed mime types are hardcoded in the class into a private property. (with no getter method). This PR solves this limitation.

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 3, 2018

CLA assistant check
All committers have signed the CLA.

/**
* @return string[]
*/
public function getAllowedMimeTypes()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need such getter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, we do not need it. i have followed the convention by $allowedExtensions

'image/gif',
'image/png',
];
protected $allowedMimeTypes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must remain private.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be private.
what is the purpose of users can extend allowedExtensions but allowedMimeTypes is hardcoded?
I think two property is connected logically.

@orlangur orlangur self-assigned this Oct 4, 2018
@tufahu
Copy link
Contributor Author

tufahu commented Oct 4, 2018

@orlangur i've answered and fixed all of your requests. thank you!

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-3157 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@tufahu thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-3157 has been created to process this Pull Request

@magento-engcom-team magento-engcom-team merged commit d42951b into magento:2.3-develop Oct 16, 2018
@magento-engcom-team
Copy link
Contributor

Hi @tufahu. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants