Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single media group type and media count validation #2924

Merged
merged 26 commits into from Oct 10, 2019

Conversation

@carakas
Copy link
Member

carakas commented Oct 8, 2019

Type

  • Non critical bugfix
  • Enhancement
  • Feature

Pull request description

Cleaned up the layout of using the media library a bit.
Screenshot 2019-10-08 at 16 59 03

After this you won't need to do anything fancy anymore in your template to be able to use the media library in your form, just add it to your form and it works.

I also implemented a way to limit the amount of media that is uploaded
Screenshot 2019-10-08 at 17 15 23
Screenshot 2019-10-08 at 17 15 19
Screenshot 2019-10-08 at 17 15 10

To make things easy for when you only need one item a form type has been added that sets the limits to 1 for you

@carakas carakas added this to the 5.7.0 milestone Oct 8, 2019
@carakas carakas requested review from forkcms/core-contributors Oct 8, 2019
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
class SingleMediaGroupType extends MediaGroupType

This comment has been minimized.

Copy link
@jeroendesloovere

jeroendesloovere Oct 8, 2019

Member

Can you use final class instead?

This comment has been minimized.

Copy link
@carakas

carakas Oct 8, 2019

Author Member

Shouldn't people be able to extend this?

This comment has been minimized.

Copy link
@ohvitorino

ohvitorino Oct 10, 2019

Contributor

I guess they could do it by using the getParent function instead. That's actually how is exemplified in the Symfony documentation.

This comment has been minimized.

Copy link
@ohvitorino

ohvitorino Oct 10, 2019

Contributor

And I guess you could also do it here as well.

This comment has been minimized.

Copy link
@StijnVrolijk

StijnVrolijk Oct 9, 2019

Contributor

In this case I see no purpose in not adding final as there is barely anything in this class. If you want to protect the "single" aspect of this class but allow the fields to be added or edited, you should use final public function configureOptions(OptionsResolver $resolver): void instead.

This comment has been minimized.

Copy link
@carakas

carakas Oct 9, 2019

Author Member

fixed in 84965db

@jeroendesloovere

This comment has been minimized.

Copy link
Member

jeroendesloovere commented Oct 8, 2019

Hi there.
Great work over here.

use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
class SingleMediaGroupType extends MediaGroupType

This comment has been minimized.

Copy link
@ohvitorino

ohvitorino Oct 10, 2019

Contributor

I guess they could do it by using the getParent function instead. That's actually how is exemplified in the Symfony documentation.

use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;
class SingleMediaGroupType extends MediaGroupType

This comment has been minimized.

Copy link
@ohvitorino

ohvitorino Oct 10, 2019

Contributor

And I guess you could also do it here as well.

@ohvitorino

This comment has been minimized.

Copy link
Contributor

ohvitorino commented Oct 10, 2019

Is this ok for the @forkcms/frontenders? It's maybe a good idea to test this locally before merging.

@carakas carakas force-pushed the justcarakas:single-media-group-type branch from dce0265 to b2ab056 Oct 10, 2019
@carakas carakas force-pushed the justcarakas:single-media-group-type branch from b2ab056 to eac5db8 Oct 10, 2019
@carakas carakas merged commit 1f68e5e into forkcms:master Oct 10, 2019
3 checks passed
3 checks passed
Scrutinizer 2 updated code elements
Details
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@carakas carakas deleted the justcarakas:single-media-group-type branch Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.