zdavatz / spreadsheet

The Ruby Spreadsheet by ywesee GmbH
http://spreadsheet.ch
GNU General Public License v3.0
1.13k stars 240 forks source link

Duplicate/repeated Spreadsheet::Formats not working in Excel 2013 for Windows #120

Closed chellberg closed 9 years ago

chellberg commented 9 years ago

Encountered an issue when using roo-xls/spreadsheet to generate a .xls file with alternating row color/shading. Each row has its style set by referencing this method:

def self.value_format(index)
  color = index.odd? ? :silver : :white
  Spreadsheet::Format.new(
    :horizontal_align => :center,
    :vertical_align   => :top,
    :shrink           => true,
    :pattern_fg_color => color,
    :pattern          => 2,
    :border           => :thin,
    :size             => 11
  )
end

The resulting file looks fine in Excel 2011 for OS X and earlier versions of Excel for Windows, but on Excel 2013/Windows 8 all formatting below row 24 is missing.

screenshot

A coworker hypothesized that this might be the result of some kind of limit on the number of repeated, identical formats in Excel 2013 and/or the way formats are applied in the spreadsheet gem. A test using this modified code

def self.value_format(index)
    color = index > 55 ? :white : "xls_color_#{index}".to_sym
    Spreadsheet::Format.new(
      :horizontal_align => :center,
      :vertical_align   => :top,
      :shrink           => true,
      :pattern_fg_color => color,
      :pattern          => 2,
      :border           => :thin,
      :size             => 11
    )
  end

produced these results (in Excel 2013, Windows 8)

image

which I believe confirms that the issue has to do with repeated/alternated, identical Spreadsheet::Formats.

zdavatz commented 9 years ago

Can you pleaes provide me with a full script via gist.github.com and also link your files here in some way. How where the XLS files created in the first place? What happens if you open the files with LibreOffice?

zdavatz commented 9 years ago

Can you please link your test code and your sample files here, so I can have a look at them. Thanks!

adamcooke commented 9 years ago

In the absence of any additional information from @chellberg, I've prepared a quick script to demo this. The gist linked below will generate the screenshot shown below. It certainly looks like a limitation in Excel 2013. It looks fine in other versions of Excel on other platforms. I hope that helps.

https://gist.github.com/adamcooke/bc0f0e687d4712073a02

Screenshot

chellberg commented 9 years ago

Sorry about that @zdavatz - I missed your replies. Thanks for putting that gist together, @adamcooke.

zdavatz commented 9 years ago

Quote @adamcooke

It certainly looks like a limitation in Excel 2013. It looks fine in other versions of Excel on other platforms.
varunDM commented 8 years ago

So there's no solution for this??

adamcooke commented 8 years ago

The solution is to cache the format instances and apply existing instances to cells using set_format.

format1= Spreadsheet::Format.new(:pattern => 1, :pattern_fg_color => :silver, :bottom => :thin, :top => :thin, :left => :thin, :right => :thin)
format2 = Spreadsheet::Format.new(:pattern => 1, :pattern_fg_color => :white, :bottom => :thin, :top => :thin, :left => :thin, :right => :thin)

worksheet.row(i).set_format(0, format1)
worksheet.row(i).set_format(1, format2)
worksheet.row(i).set_format(2, format1)
zdavatz commented 8 years ago

Ok, thanks for sharing this!

InteNs commented 5 years ago

@adamcooke this fixes the issue but isn't ideal. Is there another way where the update_format method could be used? (we draw some lines on the sheet after initial formatting, these lines will display on other cells also using the same cached format instances

InteNs commented 5 years ago

i've fixed the issue with the following code:

    def register_format(format)
      # build a kv store with the format hash as key and the format object as value
      @formats ||= {}
      unless @formats.key?(format)
        @formats[format] = ::Spreadsheet::Format.new(format)
      end
      @formats[format]
    end

    def format_cell(row_index, column_index, **format)
      row = sheet.row(row_index)

      new_format = if row.formats[column_index].present?
                     # existing format, clone it and update the clone
                     row.format(column_index).clone.update_format(**format)
                   else
                     # no format here, grab a cached or new format
                     register_format(format)
                   end

      row.set_format(column_index, new_format)
    end
zdavatz commented 5 years ago

great, thank you for reporting!

InteNs commented 5 years ago

still had some issues with even larger excel files, updated the code as follows:

    def register_format(format)
      @formats ||= {}
      unless @formats.key?(format)
        @formats[format] = ::Spreadsheet::Format.new(format)
      end
      @formats[format]
    end

    def format_cell(row_index, column_index, **format)
      row = sheet.row(row_index)

      if row.formats[column_index].present?
        # existing format, clone it and update the clone
        existing_format, obj = @formats.find { |_, v| v == row.formats[column_index] }

        if existing_format.present?
          # we found the format combination, now merge it
          format = existing_format.merge(format)

          unless @formats.key?(format)
            # if the merged format hasn't been used before, register it
            obj = obj.clone.update_format(**format)
            @formats[format] = obj
          end
        end
      end

      row.set_format(column_index, register_format(format))
    end
zdavatz commented 5 years ago

even better!