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

Add "block.admin_preview" block #7033

Merged
merged 2 commits into from
Apr 17, 2021

Conversation

phansys
Copy link
Member

@phansys phansys commented Apr 11, 2021

Add "block.admin_preview" block in order to show a preview for admin lists in dashboard

I am targeting this branch, because this feature respects BC.

Rebase from #5530.

This is an example of how this block looks like:

image

Changelog

### Added
- Added "block.admin_preview" block in order to show a preview for admin lists in dashboard

To do

  • Find a way to set "_sort_by" and "_sort_order" params out of a request context.

@phansys
Copy link
Member Author

phansys commented Apr 11, 2021

In order to avoid the workaround near the datagrid sorting, we should find a way to provide this parameter to the admin or the datagrid out of a request context.
Is there a good reason to forbid this in the way we have now?
Currently, there is no way to set this value:

if (isset($filterParameters['_sort_by']) && \is_string($filterParameters['_sort_by'])) {
if ($this->hasListFieldDescription($filterParameters['_sort_by'])) {
$filterParameters['_sort_by'] = $this->getListFieldDescription($filterParameters['_sort_by']);
} else {
$filterParameters['_sort_by'] = $this->createFieldDescription(
$filterParameters['_sort_by']
);
$this->getListBuilder()->buildField(null, $filterParameters['_sort_by'], $this);
}
}

public function setValue($name, $operator, $value)
{
$this->values[$name] = [
'type' => $operator,
'value' => $value,
];
}

@phansys phansys marked this pull request as ready for review April 11, 2021 23:30
src/Block/AdminPreviewBlockService.php Outdated Show resolved Hide resolved
src/Block/AdminPreviewBlockService.php Outdated Show resolved Hide resolved
src/Resources/views/Block/block_admin_preview.html.twig Outdated Show resolved Hide resolved
src/Resources/views/Block/block_admin_preview.html.twig Outdated Show resolved Hide resolved
tests/Block/AdminPreviewBlockServiceTest.php Outdated Show resolved Hide resolved
@franmomu
Copy link
Member

In order to avoid the workaround near the datagrid sorting, we should find a way to provide this parameter to the admin or the datagrid out of a request context.
Is there a good reason to forbid this in the way we have now?

I don't know, but 👍 to add a way to configure it out of a request context.

@core23
Copy link
Member

core23 commented Apr 15, 2021

Can you please add a screenshot how this would look like

@phansys
Copy link
Member Author

phansys commented Apr 15, 2021

Can you please add a screenshot how this would look like

Done.

core23
core23 previously approved these changes Apr 15, 2021
{% for field_description in admin.list.elements %}
{% if field_description.option('code') == '_select' %}
{# NEXT_MAJOR: Replace the previous condition with the following one #}
{#{% if field_description.option('code') == constant('Sonata\\AdminBundle\\Datagrid\\ListMapper::NAME_SELECT') %}#}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean field_description.name == constant('Sonata\AdminBundle\Datagrid\ListMapper::NAME_SELECT') ?

I think you can already use this condition

<thead>
<tr class="sonata-ba-list-field-header">
{% for field_description in admin.list.elements %}
{% if field_description.option('code') == constant('Sonata\\AdminBundle\\Datagrid\\ListMapper::NAME_SELECT') %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% if field_description.option('code') == constant('Sonata\\AdminBundle\\Datagrid\\ListMapper::NAME_SELECT') %}
{% if field_description.name == constant('Sonata\\AdminBundle\\Datagrid\\ListMapper::NAME_SELECT') %}

VincentLanglet
VincentLanglet previously approved these changes Apr 16, 2021
@phansys phansys requested a review from core23 April 16, 2021 21:29
// Setting a request to the admin is a workaround since the admin only
// can handle the "_sort_by" parameter from the query string.
$request = new Request(['filter' => $sortFilters]);
$request->setSession(new Session());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need a session here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm afraid:

public function getListMode()
{
if (!$this->hasRequest()) {
return 'list';
}
return $this->getRequest()->getSession()->get(sprintf('%s.list_mode', $this->getCode()), 'list');
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the request has a hasSession or something? We could add a check to the code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I was thinking the same.
Let me check and I'll update the PR if possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are: #7080.

franmomu
franmomu previously approved these changes Apr 17, 2021
tests/Block/AdminPreviewBlockServiceTest.php Outdated Show resolved Hide resolved
tests/Block/AdminPreviewBlockServiceTest.php Outdated Show resolved Hide resolved
franmomu
franmomu previously approved these changes Apr 17, 2021
VincentLanglet
VincentLanglet previously approved these changes Apr 17, 2021
Copy link
Member

@franmomu franmomu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased and push with --force-with-lease to use the DatagridInterface constants.

@VincentLanglet
Copy link
Member

Thanks !

@VincentLanglet VincentLanglet merged commit 9bbbab4 into sonata-project:3.x Apr 17, 2021
@phansys phansys deleted the preview_block branch April 17, 2021 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants