steps to reproduce:
1. create a table-style view exposing multiple content types
2. add fields from each content type, including at least 2 fields which only exist on one content type each
3. add "content type" field
4. group results by content type
5. assign fields from each content type to the same column
6. click "hide empty columns" in table display settings

expected behavior:
the number of result columns should match the number of header columns
"merged" fields should inherit "hide empty column" setting from the fields they're merged with

actual behavior:
if the "merged" field was not checked "hide empty column" before it was assigned to be merged with another column, it will cause an empty column to appear.
an extra result column causes the headers to appear misaligned.

for example:
create a view showing News and Events
add fields "News Date", "News Lede", "Event Start Date" and "Event End Date"
assign "News Lede" to "News Date" column
click "hide empty column" for "News Lede" in style options
assign "Event End Date" to "Event Start Date" column
click "hide empty column" for "Event End Date" in style options

result: "News" table has an errant column for "Event Start Date," presumably because it's still trying to render the empty "Event End Date" column.
Similarly, "Event" table has an errant column for "News Lede"

Let me know if it will help for me to post an exported view and/or screenshot.
I'm marking this minor because it's pretty obscure, and it can be fixed by first setting "hide empty columns" on the "merged" fields.

CommentFileSizeAuthor
#10 views-combined_columns_empty-1778106-10.patch2.1 KBanrikun
#7 views-combined_columns_empty-1778106-7.patch2.08 KBanrikun
#6 views-combined_columns_empty-1778106-6.patch1.8 KBanrikun
#5 views-combined_columns_empty-1778106-5.patch811 bytesandrew answer
#2 views_combined_columns_empty-views_plugin_style_table.inc-1778106.patch933 bytesaaronbauman

Comments

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Needs tests 
Let me know if it will help for me to post an exported view and/or screenshot.

If you can provide us a view everyone can import without additional effort like creating fieldapi fields you would make us happy.
This can often be a hard part.

I fear this could be tricky to test.

aaronbauman’s picture

aaronbauman
he/him
commented
Status: Active » Needs review
StatusFileSize
new views_combined_columns_empty-views_plugin_style_table.inc-1778106.patch933 bytes

OK, here is a pastebin of the view: http://pastebin.com/5e7jSxv5
And patch attached.
Basically, when the table style setting is submitted, this patch will ensure that if columns have been combined, child columns will inherit "empty_column" setting from their parents. I started writing this patch in views/theme/theme.inc template_preprocess_views_view_table(), but it seems like there's already enough stuff going on there.

I didn't see an obvious place to put a patch in the existing tests directory -- lemme know where you think it should go.

leanne9’s picture

leanne9 commented

Just reading this helped me solve my issue- had to go back and check the boxes before "merging" for columns.

chris matthews’s picture

chris matthews commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs reroll

The 6 year old patch to views_plugin_style_table.inc does not apply to the latest views 7.x-3.x-dev and if still applicable needs to be rerolled.

Checking patch plugins/views_plugin_style_table.inc...
error: while searching for:
    );
  }

  function even_empty() {
    return parent::even_empty() || !empty($this->options['empty_table']);
  }

error: patch failed: plugins/views_plugin_style_table.inc:292
error: plugins/views_plugin_style_table.inc: patch does not apply
andrew answer’s picture

andrew answer commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new views-combined_columns_empty-1778106-5.patch811 bytes

Patch rerolled.

anrikun’s picture

anrikun commented
Priority: Minor » Normal
StatusFileSize
new views-combined_columns_empty-1778106-6.patch1.8 KB

Patch at #5 works but requires each existing view to be re-edited.

Patch below adds changes in theme.inc to make it work for existing views too.
Besides:

  1. It fixes a buggy reuse of variable $columns. (I turn this bug into normal for that.)
  2. It gets rid of the dangerous use of reference &$columns_items.
anrikun’s picture

anrikun commented
StatusFileSize
new views-combined_columns_empty-1778106-7.patch2.08 KB

An extra fix is needed: column header gets removed as long as one child column is empty, even if others are not.
This new patch addresses this issue too.

anrikun’s picture

anrikun commented

I'm bumping this to expect the bug to be fixed in next release...

anrikun’s picture

anrikun commented

I'm bumping this again to expect the bug to be fixed in next release...

anrikun’s picture

anrikun commented
StatusFileSize
new views-combined_columns_empty-1778106-10.patch2.1 KB

Rerolled patch