The admin bean/block list is hardcoded.

It could be easily switched to a view, if all relevant handlers are provided.
Having it in views would lead to great editor experience as the lists can be easily customized.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

indytechcook’s picture

While I try not to use views on my projects, I do accept patches for enhanced functionality as long as it doesn't degrade the current experience or goals of the project.

miro_dietiker’s picture

We might provide.

Note that in our case, we see the admin view of beans is very limited.
You can't search for bean titles in a "contains" exposed field.
The order of the list is currently not satisfyingly defined.
(We end um with a paginating list of unordered beans where you never know how to find the right one...)

Also now views is in core for D8, so it's the right direction...

ytsurk’s picture

Status: Active » Needs review
FileSize
10.76 KB

there we go.
already using the handler from #1918188: Few useful views handlers for work with bean blocks

on the overview page, the view is only triggered when views module is enabled.

Berdir’s picture

+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,175 @@
+
+/**
+ * Views for the default block UI.

Should have a @file on the first line of the docblock.

+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,175 @@
+  $view = new view();
+	$view->name = 'bean_blocks_overview';
+	$view->description = '';
+	$view->tag = 'default';

No idea why the default view has tabs on most lines but not all?

+++ b/views/views_handler_field_bean_operations.incundefined
@@ -0,0 +1,54 @@
+ * @file
+ * Definition of views_handler_field_node_link.
...
+class views_handler_field_bean_operations extends views_handler_field_entity {

Comment is wrong.

Berdir’s picture

+++ b/includes/bean.pages.incundefined
@@ -62,6 +62,11 @@ function bean_add_page() {
 function bean_list() {
+
+  if( module_exists('views') ) {
+    return views_embed_view('bean_blocks_overview');
+  }
+  ¶
   $rows = array();

Instead of this, views could also simply override the path, it does define everything in hook_menu_alter() for that.

ytsurk’s picture

@berdir, thx for the feedback.
sorry for the formatting issues ....

so you would implement the bean_menu_alter method, and if views is there, override the path to show the view - right ?

miro_dietiker’s picture

Status: Needs review » Needs work
+++ b/includes/bean.pages.incundefined
@@ -62,6 +62,11 @@ function bean_add_page() {
+  ¶

Trailing spaces

+++ b/views/bean.views.incundefined
@@ -30,4 +30,12 @@ function bean_views_data_alter(&$data) {
+    'help' => t('Display all the available operations links for the block.'),

"operation links" ... It's contextual links output, not a typical static "Edit" link as e.g. common on the admin/content list.

+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,175 @@
+	$view->name = 'bean_blocks_overview';

Tabs and a lot more tabs in later lines.

+++ b/views/views_handler_field_bean_operations.incundefined
@@ -0,0 +1,54 @@
+class views_handler_field_bean_operations extends views_handler_field_entity {
...
+    if (bean_access('edit', $bean)) {
...
+      $links = menu_contextual_links('bean', 'block', array($bean->delta));

Is the naming "operations" common for such a handler?

I see it's about contextual links.. Finally you check for edit permissions and trigger all contextual links?

+++ b/views/views_handler_field_bean_operations.incundefined
@@ -0,0 +1,54 @@
+      '#title' => t('Add a destination parameter to operations links so users return to this View on form submission.'),

again, contextual / operations ... after form submission.

+++ b/includes/bean.pages.incundefined
@@ -62,6 +62,11 @@ function bean_add_page() {
+  ¶

Trailing spaces

+++ b/views/bean.views.incundefined
@@ -30,4 +30,12 @@ function bean_views_data_alter(&$data) {
+    'help' => t('Display all the available operations links for the block.'),

"operation links" ... It's contextual links output, not a typical static "Edit" link as e.g. common on the admin/content list.

+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,175 @@
+	$view->name = 'bean_blocks_overview';

Tabs and a lot more tabs in later lines.

+++ b/views/views_handler_field_bean_operations.incundefined
@@ -0,0 +1,54 @@
+class views_handler_field_bean_operations extends views_handler_field_entity {
...
+    if (bean_access('edit', $bean)) {
...
+      $links = menu_contextual_links('bean', 'block', array($bean->delta));

Is the naming "operations" common for such a handler?

I see it's about contextual links.. Finally you check for edit permissions (only) and trigger all contextual links? Do we even need that edit check?

ytsurk’s picture

the operations handler comes form commerce, so the wording should be fine.
i can change it to contextual links ... ?!

regarding permissions, i would say edit should stay.
menu_contextual_links does not check for permissions as far as I see
edit somehow indicates to me,
that operations are allowed, even thoe modules hooking in there could have own permissions (fe. translate link ..)

Berdir’s picture

Views should by default override the custom page callback if you specify the same path.

ytsurk’s picture

when adding via view display (overriding the path),
this part gets lost
Screen Shot 2013-02-28 at 3.02.22 PM.png

so - i will keep the old approach.

Berdir’s picture

You should be able to specify that it has a local task menu?

ytsurk’s picture

so I have to hook_menu_alter .. ?
http://drupal.org/node/1126762

EDIT: just making the menu entry for the view page is enough.

ytsurk’s picture

Status: Needs work » Needs review
FileSize
11.48 KB

better like this

Berdir’s picture

Status: Needs review » Needs work

Code looks good now.

Test that If you have views, then the view is automatically displayed on admin/content/blocks but you can also disable it manually if you don't want that.

About the view itself, I think we don't need bid as a column in there. And if you, in the table options, display the delete link in the operations column then that additional column and the space between the links goes away. And maybe expose an additional search field for the description? sometimes you might know the description of a block you're looking for, not the title. Unfortunately, it is AFAIK not yet possible to search in two different fields with a single exposed filter without an additional module.

ytsurk’s picture

Assigned: Unassigned » ytsurk

this is already tested. so the view shows as soon as views module is enabled. the view can be omitted via views UI and disabling it ..
think this is kind of the default behavoir.

i'll move the delete link to the operations / contextual links colums, bid sometimes can be useful.
i also give the second exposed filter a try - if an additional module is needed, i think we should limit to one !

Berdir’s picture

I know it works, what's what I meant to say with that, that I did test it.

don't think the bid is important here, you're only interested in the machine name/delta. Having two exposed filters is easy. Having a single exposed filter that does a search in two different columns is not.

ytsurk’s picture

Status: Needs work » Needs review
FileSize
12.78 KB

alright,

rearranged the view a bit:

  • label/description is now also an exposed filter.
  • more sorting
  • all labels in english
  • moved BID at the end, couldn't resist to keep it ;)
Berdir’s picture

FileSize
105.15 KB
+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,219 @@
+  $handler->display->display_options['menu']['description'] = 'Bean blocks overVIEW';

Why uppercase?

Looks good otherwise, still disagree with showing the bid column but that's something for the maintainer to decide.

Attaching a screenshot of how this currently looks.

ytsurk’s picture

alright - convinced .. : bid is removed ;)
also the VIEW joke is removed

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, time for a review from the maintainer :)

miro_dietiker’s picture

+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,206 @@
+    2 => '2',
+    1 => 0,
+    3 => 0,
+    4 => 0,
+    5 => 0,
+    6 => 0,
+    7 => 0,
+    8 => 0,
...
+    2 => '2',
+    1 => 0,
+    3 => 0,
+    4 => 0,
+    5 => 0,
+    6 => 0,
+    7 => 0,
+    8 => 0,
...
+    2 => '2',
+    1 => 0,
+    3 => 0,
+    4 => 0,
+    5 => 0,
+    6 => 0,
+    7 => 0,
+    8 => 0,

I guess these remember_roles are environment specific... Dunno how this should look like for a generic install.
API documentation of remember_roles in exposed_form.

ytsurk’s picture

Miro, you're right. thx for again taking a look !

i copied infos from my local setup into the view.

miro_dietiker’s picture

Now looks fine!

DamienMcKenna’s picture

Shouldn't this also either remove the existing 'admin/content/blocks' page or have the default view disabled so it has to be specifically enabled?

Berdir’s picture

Not necessary. If views is disabled, then it uses the default implementation (The maintainer doesn't want to depend on Views, see above) and if it's enabled, then the view is used automatically.

indytechcook’s picture

Status: Reviewed & tested by the community » Needs work

This is a similar approach that admin_views, and several other modules use.


+++ b/views/bean.views_default.incundefined
@@ -0,0 +1,158 @@
+  $view->api_version = '3.0';
+  $view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

I'd prefer this be set to TRUE so it's a passive change.

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
608 bytes
1.17 KB

Done.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, disabled by default is also fine with me.

ytsurk’s picture

Yes - i can agree too.

miro_dietiker’s picture

Great, ready then.

miro_dietiker’s picture

I just have realized that the patch of Albert Volkman is missing the new files introduced by ytsurk.

Albert Volkman’s picture

D'oh. Thanks for the catch @miro_dietiker!

fmitchell’s picture

Assigned: ytsurk » fmitchell

Assigning to me to bring in

miro_dietiker’s picture

Status: Reviewed & tested by the community » Needs work

From discussion with maintainers:
We should add a hint about new views integration to README.txt and how to enable.

fmitchell’s picture

DamienMcKenna’s picture

Status: Closed (fixed) » Fixed

Thanks for everyone's work on this.

BTW it's customary to leave the issue in 'fixed' status after the commit.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.