Rework form options, download handler options #700
Conversation
Resources/doc/form/vich_file_type.md
Outdated
| @@ -1,4 +1,4 @@ | |||
| VichFileType | |||
| VichFileType Field | |||
| ============ | |||
There was a problem hiding this comment.
You should adapt length here to match title length
Koc
force-pushed
the
minior-fixes
branch
6 times, most recently
from
3acf823
to
762cbab
Compare
|
@garak are minior bc-breaks in form types template acceptable for 1.6.0? |
|
Also I can add integration with LiipImagineBundle for |
|
Please elaborate about BC breaks and LiipImagineBundle |
|
BC-break - I've removed Integration: we can add option like |
|
We should keep the variable, even if unused, to avoid breaking bad (I mean, causing errors). If the only break is a change if behaviour, it could be feasible. |
Koc
force-pushed
the
minior-fixes
branch
6 times, most recently
from
1b121e1
to
e92845e
Compare
| ```php | ||
| use Vich\UploaderBundle\Form\Type\VichFileType; | ||
|
|
||
| $builder->add('genericFile', VichFileType::class, [ |
There was a problem hiding this comment.
copy-paste error, need to be VichImageType
Form/Type/VichFileType.php
Outdated
| $resolver->setAllowedTypes('error_bubbling', 'bool'); | ||
|
|
||
| $downloadUriNormalizer = function (Options $options, $downloadUri) { | ||
| if (isset($options['download_link'])) { |
There was a problem hiding this comment.
need comparation with null, isset not works as expected
There was a problem hiding this comment.
What if key is not set? Maybe an empty could be better?
There was a problem hiding this comment.
It always set because of setDefault call
Koc
force-pushed
the
minior-fixes
branch
5 times, most recently
from
6fdeabd
to
41f97cf
Compare
Koc
changed the title
|
@garak tests pass, all new cases are covered with tests, docs updated, please review and merge. |
Form/Type/VichFileType.php
Outdated
| class VichFileType extends AbstractType | ||
| { | ||
| /** |
There was a problem hiding this comment.
Is there a reason for removing phpdoc blocks here (and in the following properties/methods)?
There was a problem hiding this comment.
This blocks are duplication of real typehints in constructor so there is no any benefit from they IMHO.
There was a problem hiding this comment.
I'm not sure that every IDE is smart enough to pass the type after assignment. Anyway, phpdoc block could be used for other than autocompletion in IDEs (e.g. for doc auto-generation).
Anyway, it would be better to keep consistency between files (so, since other files do use phpdoc blocks, this file should use it as well)
…ation for image form type. Fix phpdocs for interfaces
Notable changes:
VichFileType:download_linkdeprecated withdownload_uri, which supports callableVichFileType:download_labelnow supportstrue(use original file name),callableand instances ofPropertyPathVichFileType: unifiedtranslation_domainoption usageVichImageType: added optionimage_uriVichImageType: added optional integration with LiipImagineBundletrue(use original file name) as served file nameStorageInterface,UploaderHelper,DownloadHandler