TASK: Added many properties to EXIF/IPTC DTOs #4
Conversation
| @@ -13,7 +13,10 @@ | |||
|
|
|||
| use Doctrine\Common\Collections\ArrayCollection; | |||
|
|
|||
| /** | |||
| * ArrayCollection for meta data | |||
There was a problem hiding this comment.
Comments always tend to get outdated. So if a comment doesn't add any information, just leave it out.
There was a problem hiding this comment.
Without comments it looks kind of unfinished. But you're probably right.
| @@ -13,21 +13,21 @@ | |||
|
|
|||
| use TYPO3\Flow\Utility\Arrays; | |||
|
|
|||
| /** | |||
| * AbstractMetaDataDto | |||
There was a problem hiding this comment.
See comment about comments above ;)
| * | ||
| * @param array $properties | ||
| */ | ||
| public function __construct(array $properties) |
There was a problem hiding this comment.
Why do you think defining the default properties in the constructor is better than just defining it in the array?
There was a problem hiding this comment.
tbh: because of the ClassOverridesFieldOfSuperClassInspection of https://github.com/kalessil/phpinspectionsea#inspections-lists-architecture ;)
I can't really decide on which one is better. :/
| */ | ||
| class Exif extends AbstractMetaDataDto | ||
| { | ||
| /** | ||
| * EXIF MetaData constructor |
There was a problem hiding this comment.
See comment about comments above ;)
| */ | ||
| class Iptc extends AbstractMetaDataDto | ||
| { | ||
| /** | ||
| * @var array | ||
| * IPTC MetaData constructor |
| @@ -14,12 +14,16 @@ | |||
| use Neos\MetaData\Domain\Collection\MetaDataCollection; | |||
| use TYPO3\Media\Domain\Model\Asset; | |||
|
|
|||
| /** | |||
| * Meta Data Mapper Interface | |||
|
|
||
| /** | ||
| * Meta Data Manager |
| # tags: '${asset.Tags}' |
There was a problem hiding this comment.
Not bad to comment that one out. But why not include the keywords?
There was a problem hiding this comment.
I wanted to show what is happening by default. Nothing but runtime would change by uncommenting that line.
| * **EXIF:** Exchangeable image file format for digital still cameras, the technical meta data of an image. For further specifications see: [http://www.exif.org/Exif2-2.PDF](). | ||
| * **Asset**: The asset DTO provides basic data about the asset, like original file name. | ||
| * **IPTC**: Image meta data, title and description of the photograph and the author. For further specifications see: [https://www.iptc.org/std/photometadata/specification/IPTC-PhotoMetadata]() | ||
| * **EXIF**: Exchangeable image file format for digital still cameras, the technical meta data of an image. For further specifications see: [http://www.exif.org/Exif2-2.PDF](). |
There was a problem hiding this comment.
| 'YResolution' => 0.0, | ||
| 'ResolutionUnit' => '', | ||
| // 'StripOffsets' => ?, | ||
| // 'RowsPerStrip' => ?, |
There was a problem hiding this comment.
If we don't want to support these, we should leave them out. Looks somehow unfinished.
There was a problem hiding this comment.
yeah. I don't think they will be used and it wasn't immediately clear to me how to represent these. Do you have an idea? If not they can be removed entirely.
| @@ -83,7 +103,7 @@ public function mapMetaData(Asset $asset, MetaDataCollection $metaDataCollection | |||
| $contextVariables = array_merge($this->defaultContextVariables, $metaDataCollection->toArray()); | |||
|
|
|||
| if (isset($this->metaDataMappingConfiguration['title'])) { | |||
| $asset->setTitle((string)EelUtility::evaluateEelExpression($this->metaDataMappingConfiguration['title'], $this->eelEvaluator, $contextVariables)); | |||
| $asset->setTitle(substr((string)EelUtility::evaluateEelExpression($this->metaDataMappingConfiguration['title'], $this->eelEvaluator, $contextVariables), 0, 255)); | |||
There was a problem hiding this comment.
But it shouldn't be adjustable since the title cannot be longer than 255 chars (see TYPO3\Media\Domain\Model\Asset).
* removed some unnecessary comments * override parent property `properties` instead setting it in constructor * removed comments of EXIF properties not implemented * updated link to EXIF specification in ReadMe
also improved code style and fixed a possible bug.