All columns are left aligned (aren't they?), but sometimes you'd like to center- or right-align them. So it would be a nice feature.

Comments

merlinofchaos’s picture

Status: Active » Closed (won't fix)

At this time this is left exclusively up to CSS. You have to do that in your theme. Views doesn't add CSS for you.

itrivino’s picture

But, does Views module apply some sort of class to each column, so I could do that with CSS?

merlinofchaos’s picture

Yes, it does.

bendiy’s picture

Is there a chance that this could go into Views 3? There is a big need to right align numeric columns and currency amounts.

I understand the need to separate content from design. However, having to edit the CSS file every time you create a View is a hassle. The CSS class on each column is unique to the column name, so you have to have a CSS entry for each column name on each View you want to change the alignment on. That's a lot of editing and maintaining.

The Views Calc module does a great job of giving a column alignment option, but it would be nice to have it's "Justification" option on the standard Views Table style.

I've migrated the Views Calc Justification setting to Views' Table style. I would like to submit a patch for this if you're open to it and I have time to port it to the latest Dev release.

bendiy’s picture

Component: table style » Code
Status: Closed (won't fix) » Needs review

I know this was marked as won't fix, but I'm submitting this patch for review. See my comments above.

The patch applies to Views 6.x-2.8.

I'm not sure why the patch wants to override all of the includes/admin.inc file, but here are the two lines that were changed in the admin.inc file:

/**
 * Theme the form for the table style plugin
 */
function theme_views_ui_style_plugin_table($form) {
  $output = drupal_render($form['description_markup']);

  $header = array(
    t('Field'),
    t('Column'),
+    t('Justify'),
    t('Separator'),
    array(
      'data' => t('Sortable'),
      'align' => 'center',
    ),
    array(
      'data' => t('Default sort'),
      'align' => 'center',
    ),
  );
  $rows = array();
  foreach (element_children($form['columns']) as $id) {
    $row = array();
    $row[] = drupal_render($form['info'][$id]['name']);
    $row[] = drupal_render($form['columns'][$id]);
+    $row[] = drupal_render($form['info'][$id]['justify']);
    $row[] = drupal_render($form['info'][$id]['separator']);
    if (!empty($form['info'][$id]['sortable'])) {
      $row[] = array(
        'data' => drupal_render($form['info'][$id]['sortable']),
        'align' => 'center',
      );
      $row[] = array(
        'data' => drupal_render($form['default'][$id]),
        'align' => 'center',
      );
    }
    else {
      $row[] = '';
      $row[] = '';
    }
    $rows[] = $row;
  }

  // Add the special 'None' row.
  $rows[] = array(t('None'), '', '', '', array('align' => 'center', 'data' => drupal_render($form['default'][-1])));

  $output .= theme('table', $header, $rows);
  $output .= drupal_render($form);
  return $output;
}

Please review.

Thanks!

bendiy’s picture

StatusFileSize
new206.47 KB

Here's the patch...

dawehner’s picture

Status: Needs review » Needs work

Can you please update your local views, and try to apply your short difference?

A patch of 200KB for this kind of stuff is funny :)

The line endings of some files where changed.

bendiy’s picture

Status: Needs work » Needs review
StatusFileSize
new4.35 KB

I believe it's fixed now. See the attached patch.

dagmar’s picture

Version: 6.x-2.3 » 6.x-2.x-dev
Status: Needs review » Needs work
+++ sites/all/modules/views/theme/views-view-table.tpl.php	10 Dec 2009 22:44:24 -0000
@@ -22,7 +22,7 @@
+        <th class="views-field views-field-<?php print $fields[$field]; ?> views-<?php print $options['info'][$field]['justify'] ?>">

You have to clean up the output. http://drupal.org/writing-secure-code

+++ sites/all/modules/views/plugins/views_plugin_style_table.inc	10 Dec 2009 22:44:24 -0000
@@ -222,6 +222,18 @@
+        '#default_value' => isset($this->options['info'][$field]['justify']) ? $this->options['info'][$field]['justify'] : 'justify-none',
+        '#options' => array(
+          'justify-none' => t('None'), 

Is 'justify-none' class neccesary if there isn't aligment defined?

Why you don't simply put '' => t('None')

bendiy’s picture

StatusFileSize
new4.36 KB

@#9 dagmar, thanks for the feedback. You are correct.

I've wrapped the output in check_plain() and changed the 'justify-none' to ''. I also moved the 'views-' part of the CSS class into the views_plugin_style_table.inc file so it doesn't print if 'None' is selected.

Please review the attached.

Thanks!

bendiy’s picture

Status: Needs work » Needs review

Changing status to "needs review".

dagmar’s picture

Status: Needs review » Needs work
+++ sites/all/modules/views/theme/views-view-table.tpl.php	10 Dec 2009 22:44:24 -0000
@@ -32,7 +32,7 @@
Index: sites/all/modules/views/includes/admin.inc

From: http://drupal.org/patch/create

Contributed module/theme patches:
Run the diff command in the module or theme's root directory. (Example: /sites/all/modules/foobar).

Sorry, you must create patches from views directory. I don't have my views directory in sites/all/modules so this patch doesn't apply.

+++ sites/all/modules/views/theme/views-view-table.tpl.php	10 Dec 2009 22:44:24 -0000
@@ -22,7 +22,7 @@
+        <th class="views-field views-field-<?php print $fields[$field]; ?> <?php print check_plain($options['info'][$field]['justify']) ?>">

You should use a variable populated in theme.inc instead of the check_plain here.

You can define a new variable named i.e. $fields_alings, and use this to define css class alingment. Also, provide a new line in the tpl.php file to let know themers what is for.

+++ sites/all/modules/views/plugins/views_plugin_style_table.inc	10 Dec 2009 22:44:24 -0000
@@ -222,6 +222,18 @@
+        '#default_value' => isset($this->options['info'][$field]['justify']) ? $this->options['info'][$field]['justify'] : '',

Use !empty() instead of isset().

Bilmar’s picture

subscribing

bendiy’s picture

Status: Needs work » Needs review
StatusFileSize
new3.55 KB

@#13 dagmar - Thanks for the coaching.

I've created a new patch to address all of your issues. This is against Views 2 DRUPAL-6--2 version.

List of changes:

  1. Patch should apply to the root of the Views directory now. No more sites/all/modules/views.
  2. I saw no need to create a variable in theme.inc. I appended my CSS class to the existing $var['fields'][$field] array value inside of theme.inc. This eliminated the need to edit the views-view-table.tpl.php file. I think this is a cleaner and more correct solution.
  3. I'm now using !empty()
  4. I changes the wording across the patch from "justify" to "align". I think this makes more sense.

I'm not sure if views_css_safe() is the correct function to use or if I should be using check_plain() instead in theme.inc.

Please review.

Thanks!

dagmar’s picture

Excellent!, only two minors things.

Maybe is better put the options in this order:

Left
Center
Right

And in line 87 you should erase the white spaces.

Please name the pacth views-383994.patch next time.

After that you will get my RTBC. Nice job!

merlinofchaos’s picture

My stance on this kind of thing has softened, and for tables, column align seems like an acceptable field in tables. Keep up the work on this patch.

I am not sure if it will be considered for 2.x, however (I know, I'm a pain), because with 3.0 in full swing, we are trying to limit changes to the 2.x branch to be critical features and bug fixes only.

bendiy’s picture

StatusFileSize
new3.58 KB

@#15

Attached is a, hopefully, final patch. I've change the order to Left, Center, Right. I've removed the white spaces.

@#16

It would be nice to see. If it's approved, hopefully it's not too late. I will port this to Views 3.0 in the future, but it will probably be a few weeks.

dagmar’s picture

Version: 6.x-2.x-dev » 6.x-3.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: +alpha-2 blocker

The patch applies for views 3, there is not necessary a reroll.

patching file css/views.css
patching file includes/admin.inc
Hunk #1 succeeded at 3333 (offset 303 lines).
Hunk #2 succeeded at 3349 (offset 303 lines).
patching file plugins/views_plugin_style_table.inc
patching file theme/theme.inc
Hunk #1 succeeded at 330 (offset -8 lines).

Nice work @bendiy.

bendiy’s picture

Great. Thanks!

merlinofchaos’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 3.x branch for D6 and D7.

Status: Fixed » Closed (fixed)

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

dawehner’s picture

Issue tags: -alpha-2 blocker

remote tag