Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Issue tags: +views 3.x roadmap

Add Tag.

merlinofchaos’s picture

My immediate thought is that the items in row-style-options.jpg could actually be on the fields themselves? That way *all* styles could support that?

merlinofchaos’s picture

Secondly, what I think would be most useful is if the semantic options would mostly be on the base row/style classes, and a 'semantic' setting could be in the plugin definition which would make it very simple to let plugins say "I support the semantic rules" and turn on the UI for those. One of the problems people have experienced with the semantic style is that they get very sad when they want to use semantic options with a table and they cannot.

Anonymous’s picture

merlinofchaos:

My instinct was also to put the element and class attributes on the fields as well, but I decided against it for a few reasons.

1. UI. Picking the tags and classes is easiest when you can see and change all of the fields together.

2. Inheritability. Fields, sorts, arguments and relationships tend to be the things I don't override as much as I override display, style and row style options.

3. Hierarchy. As an example, think of a view with a page display and a block display. By default, Drupal themes block titles with h2.title. Node teasers are also themed with h2.title. If I want to use fields for both of these displays, I may want to use h3.title around node titles in the block display and use h2.title or node row style with teasers on the page display.

But it also makes sense to put this onto field handlers. A lot of rewriting gets done there already, too, with links, altering output and token replacement.

Thanks dereine for starting this work!

Anonymous’s picture

Yar! Merlinofchaos knows best. I'm fiddling with a view right now and I now see the indisputable merit of his idea.

The view has 3 displays. Each has one field, but in each display the field is different (field_image, field_video, field_audio). Without overriding the row style options on each display, the output is always broken for TWO of the displays. The field names are used in the semantic views row style options for the default display, but the field name for the default is only valid for one of the displays. The other displays are looking for different field names in the row style options!

Sheesh now I'm consternated. I need to document this in Semantic Views.

merlinofchaos’s picture

Status: Needs review » Needs work
q0rban’s picture

subscribe.

dawehner’s picture

Even this is not a big argument but i suggest to not use this on field level, too, because i wouldn't expect it there.

For me everything beside the left column is related to the sql query. The left is for settings and style and some general query settings.

Anonymous’s picture

Dereine and I have arranged to meet up in IRC on Saturday at 1500 GMT. That's 1000 (10 a.m.) Chicago time. We'll be in #drupal-views. Please join us for a brainstorming session... this is a tricky issue to figure out, so any surplus brains will help.

q0rban’s picture

I don't think I'll be able to make that session, but my 2 cents: I think it makes sense to be at both the row level and the field level. I'm torn about whether this should actually be in the views UI though, as there are already so many options there. It would have to be done differently than it currently exists in semanticviews though, b/c having all that stuff exist at the field level for every field will just be insane.

Going to be thinking about this.

dawehner’s picture

Damn

I was sure you wanted to change the 1800 GMT

I'm sorry

Anonymous’s picture

So far, the best ideas seems to be to return to the unformatted row style that I formed the basis of Semantic Views.

I'm pasting the default views view fields template from views 2 here:

<?php foreach ($fields as $id => $field): ?>
  <?php if (!empty($field->separator)): ?>
    <?php print $field->separator; ?>
  <?php endif; ?>

  <<?php print $field->inline_html;?> class="views-field-<?php print $field->class; ?>">
    <?php if ($field->label): ?>
      <label class="views-label-<?php print $field->class; ?>">
        <?php print $field->label; ?>:
      </label>
    <?php endif; ?>
      <?php
      // $field->element_type is either SPAN or DIV depending upon whether or not
      // the field is a 'block' element type or 'inline' element type.
      ?>
      <<?php print $field->element_type; ?> class="field-content"><?php print $field->content; ?></<?php print $field->element_type; ?>>
  </<?php print $field->inline_html;?>>
<?php endforeach; ?>

The most viable sane idea so far is to expose these properties through the field handler options:

element_type
class

This would have the effect of allowing any output style to be "semantified" -- tables, grids, lists, etc.

Current Semantic Views features would be re-created by overriding the views-view-fields.tpl.php and removing the outer wrapper and the separator.

  <?php if (!empty($field->separator)): ?>
    <?php print $field->separator; ?>
  <?php endif; ?>

  <<?php print $field->inline_html;?> class="views-field-<?php print $field->class; ?>">
...
  </<?php print $field->inline_html;?>>

I have mixed feelings about this, but I think the benefit of semantifying more output styles is a good one. I also don't have a more reasonable proposal...

dawehner’s picture

Status: Needs work » Needs review
FileSize
3.68 KB

There are two hard tasks

* Don't brake existing templates/css
* Keep the inline feature

So what i did:
* i just changed the class of the wrapper of the content of the field.
* Everything else happens on the preprocess, and in the field handler.

dawehner’s picture

FileSize
4.16 KB

Integrate semantic with table style.

Minor cleanups.

This does only introduce the fields class, not the row classes.

Any idea/comment is welcomed, beside +1/subscribing :)

BenK’s picture

Very cool. Sorry to do a subscribe, but I need to keep track of this thread! :-)

dawehner’s picture

Status: Needs review » Needs work

+      return $this->options['element type'];

This has to be checked for security, but the patch could still get a review.

achton’s picture

Subscribing. So important patch, this.

dawehner’s picture

FileSize
4.16 KB

Before:

<div class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field-nid">
          <label class="views-label-nid">
        Nid:
      </label>
                <span class="field-content ">1</span>
  </div>
  </div>

After:

<div class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field-nid">
          <label class="views-label-nid">
        Nid:
      </label>
                <bar class="field-content foo-class">1</span>
  </div>
  </div>

This is not perfect.

Updated patch: Fix storage of the element

dww’s picture

Apparently, this is the better solution to what I was trying to accomplish with #918782: Allow CSS class specifically for the list markup in the list style plugin. Haven't looked at any patches, but the concept sounds good. ;)

klonos’s picture

subscribing...

tim.plunkett’s picture

Sub.

lpalgarvio’s picture

support all kinds of Styles and Row Styles like was said before...
to avoid the problem of inheritance, placement, etc, put the Semantic styling inside a new UI entry, the Field Styles - this would include the styling put on Style (View Style) and the styling put on Row Style.

additionally, rename Style to Views Style to avoid confusions.

Style settings would contain:
View Style
Row Style
Field Style --> containing all the semantic view styling.
CSS class
Theme

would also require removing any custom setting of CSS/class styling for Fields (since they would be now in Field Style).

dagmar’s picture

Status: Needs work » Needs review
FileSize
5.66 KB

After review the patch, I would like to share this code, It is a bit different from #18, due it allows users to use [replacements] inside classes (like 'rewrite the output of this field').

This may be a performance problem, and we should take care about this if this patch is included.

Related to #13

Do not brake existent features.

Ok, I agree, however new semantic capabilities may require users recreate their templates. I mean, this update should not destroy existent templates, but semantic classes will only be available for default views 3 templates. (and new overrides)

Related to #14

This does only introduce the fields class, not the row classes.

In my opinion, allow [tokens] for row classes would be great, however I don't see a clear way to do this, but maybe we can do some rework on tokens management of views to achieve this task. Also we need a new UI for this task.

Related to #16: I included a check_plain for the element_type.

Changing status to get some new reviews.

dawehner’s picture

Cool to see some progress

+      $tokens = $this->get_render_tokens($fake_item);

We could save some time here when we store the tokens per row in a static.

What about renaming semantic_cells to semantic_classes? This would help a bit to understand it better.

dagmar’s picture

Marking #945326: Extend field grouping class naming as a duplicate of this issue. There are some nice ideas too.

BrightBold’s picture

Subscribing, as my Semantic Views 7.x joy was just dashed by realizing that the view I wanted to semantify is draggable, and I have to choose one or the other. So it would be great to have this kind of integration.

dawehner’s picture

* Fixed the concatenation of the classes for tables.
* Manual testing works fine.
* Caching the render tokens isn't that easy as i thought.

merlinofchaos’s picture

Priority: Normal » Critical

Upping to critical. This is the next alpha blocker in line.

I expanded significantly on this patch:

  • For the basic 'fields' row style, nearly complete control over all markup is now in the UI. The template has been degraded to doing nearly nothing. It should remain backward compatible with old templates so it should not screw up old templates, though old templates obviously will not get new functionality.
  • Added full control to the labels and their markup.
  • Added row classes that can use tokens to table, list and unformatted. Grid did not get this. It's not an easy one to work on. Perhaps this still needs to be added.
  • Added more control to the list markup and wrapper.

I made the element selectable from a list, rather than something you enter. I think it's okay to limit to a basic set of elements. The classes are still selectable.

merlinofchaos’s picture

FileSize
32.52 KB
dawehner’s picture

FileSize
32.06 KB

Manual testing worked fine: labels, wrapper classes, row classes for style plugins.

I like the fieldsets around the field settings, it gives a strong visual "hook".

Reading the patch didn't offered something, beside it's genius.

Converted grid in the patch.
Perhaps the title/description of the row-class field for grids should be converted to "column" to describe the behavior a bit better.

dagmar’s picture

Awesome to see this. I have tested the patch and it seems to be working fine. There are some inconsistencies when Row Style is not 'Fields':

I mean, if you select 'Node' as row style, the option 'row class' in the style options should be hidden, due if you put row-[nid], the replacement will not be performed.

dagmar’s picture

Ok, after talk with dereine via IRC, forget what id said in #31.

For other side, argument replacements are not working.

In the attached patch, basically I have changed all the:

if (strpos($value, '[') !== FALSE) {

with

if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
merlinofchaos’s picture

+ * - no semantic: If TRUE, it disables the semantic output.

Documentation doesn't match reality here.

+  /**
+   * Return an HTML element based upon the field's element type.
+   */
+  function element_label_type($none_supported = FALSE, $default_empty = FALSE) {

Cut & paste documentation not matching reality.

+      if ($object->element_type) {
+        $class = '';
+        if ($object->handler->options['element_default_classes']) {
+          $class = 'field-content';
+        }
+
+        if ($classes = $object->handler->element_classes()) {
+          if ($class) {
+            $class .= ' ';
+          }
+          $class .=  $classes;
+        }
+
+        $field_output = '<' . $object->element_type . ' class="' . $class . '">' . $field_output . '</' . $object->element_type . '>';
+      }

I did this a bunch. I think it would be cleaner to use classes arrays and implode at the end, which would prevent needing to have the if checks on ' '.

Also on several of these it's possible to have no class at all, so we need to be sure to check that possibility in each case. This is now true on the field, field label and field wrapper, and also true on table field and field and field wrapper.

Can someone test this using old templates? While the new functionality should not work, the old templates should continue to function as they already have. Can anyone verify this is true for me? Just copy views-view-fields and views-view-table templates (and I guess list and unformatted and grid) into your theme directory before applying the patch and be sure to clear cache.

merlinofchaos’s picture

Version: 6.x-3.x-dev » 7.x-3.x-dev
Status: Needs review » Patch (to be ported)
FileSize
35.19 KB

Cleaned up some code comments. After review, I think this is in pretty good shape.

ericduran’s picture

This is pretty cool.

We can also add a couple of the new tags in the get_elements function. Maybe header, footer and section. Section seems like the most generic one that can be added if anything.

dawehner’s picture

Status: Patch (to be ported) » Fixed

What about adding a variable of the availible elements?

* tested unformatted style
* tested table style
* tested grid style
* tested wrapper element
* tested label element
* tested element
* tested wrapper element-class
* tested label element-class
* tested element-class

So this looks fine.

merlinofchaos’s picture

section is an HTML 5 element. I'm still not sure how we're going to handle HTML 5, but I would like to have a unified way of doing so before I start adding anything HTML 5 to Views.

Other X-HTML elements that make sense can be added to the list trivially.

klonos’s picture

Is this committed to both branches (6.x & 7.x)?

bcn’s picture

klonos’s picture

Thanks for taking the time to reply Noah ;)

jponch’s picture

subscribe.

Status: Fixed » Closed (fixed)
Issue tags: -views 3.x roadmap

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