-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
[MediaBundle] Paginate Image thumbnail view to 24 images (6 rows of 4) per page #1633
[MediaBundle] Paginate Image thumbnail view to 24 images (6 rows of 4) per page #1633
Conversation
delboy1978uk
commented
Nov 6, 2017
Q | A |
---|---|
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Fixed tickets | #1632 |
@@ -113,12 +120,24 @@ public function canDelete($item) | |||
/** | |||
* @return int | |||
*/ | |||
public function getLimit() | |||
public function getLimit() : int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC break for PHP version lower than 7.0
* @param int $limit | ||
* @return MediaAdminListConfigurator | ||
*/ | ||
private function setLimit(int $limit): MediaAdminListConfigurator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is branching from 5.0, which is targeting PHP 7. I asked @Numkil before I committed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understand but in composer we have version PHP >=5.6.0 so this case create BC break. In milestones on version 6.0 (#1445) we start using PHP 7.1 version skipped PHP 7.0 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i'll remove the return types then :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank You, nice work! :)
@@ -51,6 +54,10 @@ public function __construct( | |||
|
|||
$this->folder = $folder; | |||
$this->request = $request; | |||
|
|||
// Thumbnail view should display 24 images, list view 250 | |||
$viewMode = $request->get('viewMode'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When no view mode is in the request query, it will still display 250 items but the default viewMode is thumbnail.
@@ -32,6 +32,9 @@ class MediaAdminListConfigurator extends AbstractDoctrineORMAdminListConfigurato | |||
*/ | |||
private $request; | |||
|
|||
/** @var int $limit */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the doc on multiple lines like the variables above?
* @param int $limit | ||
* @return MediaAdminListConfigurator | ||
*/ | ||
private function setLimit($limit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a setter for this ? Can't we just do $this->limit = 24 ?
Also @delboy1978uk , this is no BC, so master branch 😉 |
Rebasing to master, will push up shortly! |
303d180
to
82726c2
Compare
rebased! 👍😀 |
…nstmaan#1633) [MediaBundle] Paginate Image thumbnail view to 24 images (6 rows of 4) per page
* master: (33 commits) [NodeBundle] vixed error bubbling for URLChooser [UtilitiesBundle] Using Symfony Process in Shell Class [RedirectBundle] Stop duplicate origins for single domain redirects [FormBundle] Make label non nullable in abstract form page part and assert not blank [AdminBundle] Hide wrapping content of large value fix styling of reset password form [GeneratorBundle]: fix classes in fields.html.twig [RedirectBundle] Ability to redirect for eg. /whatever/* to /something-else/* [UtilitiesBundle] Cipher deprecations (#1673) [MediaBundle] - Use JsonResponse to set headers instead of calling header() [FormBundle] Default file upload label required to false [MediaBundle] Include filters even if no results found [AdminBundle] remove hard coded 'validators' from twig view version bump moment js [MultiDomainBundle] Cache previously fetched node translations in array property [SearchBundle] add punctuation to kuma_ngram tokenizer Paginate Image thumbnail view to 24 images (6 rows of 4) per page (#1633) Remove alias functions usage by using the original functions Remove double (unnecessary) semicolon 'static::incrementString(...)' should be used instead. ...