Conversation
boudewijn-zicht
left a comment
boudewijn-zicht
left a comment
There was a problem hiding this comment.
Twee kleine opmerkingen, niets blocking wat mij betreft
| use Symfony\Component\Form\FormBuilderInterface; | ||
| use Symfony\Component\Form\AbstractType; | ||
| use Symfony\Component\Translation\TranslatorInterface; | ||
| use Symfony\Component\Validator\Constraints as Assert; |
There was a problem hiding this comment.
Zou dit gewoon Constraints noemen, niet Assert. Lijkt me alleen maar verwarrend.
Verder gebruik je allen de NotBlank, dus je kan ook alleen die hier importeren.
There was a problem hiding this comment.
Dit is hoe Symfony het zelf doet en ik heb mijzelf dit ook aangeleerd 🤷🏻♂️
There was a problem hiding this comment.
https://github.com/search?l=PHP&q=org%3Azicht+NotBlank&type=Code
Als je in de code van Zicht zoekt, dan is 'Assert' in de minderheid, op pagina 8 kom ik het voor het eerst pas tegen. Dus binnen zicht is het blijkbaar niet zo gebruikelijk...
Dit is voor mij niet blocking, maar persoonlijk zou ik het doen zoals de meeste code van Zicht het nu lijkt te doen. Your choice 👍
There was a problem hiding this comment.
Ik heb het ook nog nooit zo gedaan eigenlijk, ik zet hier altijd direct new NotBlank() neer
Added: - Added `region` option to ContentItemTypeType to show types for a specific region only - Introduced `HasContentItemInterface` interface to indicate an item which has a (child) Content Item (e.g. for Global Content Items used in projects) Fixed: - ContentItemTypeType choice keys and values (flipped since Sf 3) - Fixed types in ContentItemContainer, ContentItemMatrix - Use `choice.content_item_region.X` translates for content item region choices - Skip base class in DiscriminatorMapType choices + optimized code - Fixed content item edit link: made it into button and got the wrench icon back again Changed: - Use early returns in code to optimize the code and its readability
| use Symfony\Component\Form\FormBuilderInterface; | ||
| use Symfony\Component\Form\AbstractType; | ||
| use Symfony\Component\Translation\TranslatorInterface; | ||
| use Symfony\Component\Validator\Constraints as Assert; |
There was a problem hiding this comment.
https://github.com/search?l=PHP&q=org%3Azicht+NotBlank&type=Code
Als je in de code van Zicht zoekt, dan is 'Assert' in de minderheid, op pagina 8 kom ik het voor het eerst pas tegen. Dus binnen zicht is het blijkbaar niet zo gebruikelijk...
Dit is voor mij niet blocking, maar persoonlijk zou ik het doen zoals de meeste code van Zicht het nu lijkt te doen. Your choice 👍
| /** | ||
| * Interface to indicate that an object (= parent) has a (child) ContentIntemInterface | ||
| */ | ||
| interface HasContentItemInterface |
There was a problem hiding this comment.
Ik zou dit direct in de CC verwerken als deze PR gemerged, anders gaat niemand dit weten/gebruiken. Verder gebruiken we nog zelden globale contentitems in team 1, is dat voor team 2 anders? Anders is deze interface niet zo nuttig denk ik
3 participants
Added:
regionoption to ContentItemTypeType to show types for a specific region onlyHasContentItemInterfaceinterface to indicate an item which has a (child)Content Item (e.g. for Global Content Items used in projects)
Fixed:
choice.content_item_region.Xtranslates for content item region choicesChanged: