From 2bc9c3adb927c0c96463b4401455faec3142e14f Mon Sep 17 00:00:00 2001 From: Michael Dotterer Date: Wed, 16 Aug 2017 15:48:38 -0400 Subject: [PATCH 1/3] Apply border changes to all cells in a merged cell for more intuitive border setting when using merged cells --- lib/rubyXL/convenience_methods.rb | 15 ++++- lib/rubyXL/objects/worksheet.rb | 3 + spec/lib/cell_spec.rb | 98 +++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/lib/rubyXL/convenience_methods.rb b/lib/rubyXL/convenience_methods.rb index fe0db840a..cd9dd7a0b 100644 --- a/lib/rubyXL/convenience_methods.rb +++ b/lib/rubyXL/convenience_methods.rb @@ -858,7 +858,20 @@ def change_text_wrap(wrap = false) def change_border(direction, weight) validate_worksheet - self.style_index = workbook.modify_border(self.style_index, direction, weight) + merged_cell = worksheet.merged_cells && worksheet.merged_cells.detect { |mc| mc.cover?(row, column) } + cells = [] + if merged_cell + merged_cell.ref.row_range.each do |r| + merged_cell.ref.col_range.each do |c| + cells << worksheet.add_cell(r, c, '', nil, false) + end + end + else + cells << self + end + cells.each do |cell| + cell.style_index = workbook.modify_border(cell.style_index, direction, weight) + end end def change_border_color(direction, color) diff --git a/lib/rubyXL/objects/worksheet.rb b/lib/rubyXL/objects/worksheet.rb index 162ebdb09..f71c12d35 100644 --- a/lib/rubyXL/objects/worksheet.rb +++ b/lib/rubyXL/objects/worksheet.rb @@ -100,6 +100,9 @@ class TableParts < OOXMLContainerObject class MergedCell < OOXMLObject define_attribute(:ref, :ref) define_element_name 'mergeCell' + def cover?(r,c) + ref.row_range.cover?(r) && ref.col_range.cover?(c) + end end # http://www.datypic.com/sc/ooxml/e-ssml_mergeCells-1.html diff --git a/spec/lib/cell_spec.rb b/spec/lib/cell_spec.rb index ad92566dc..f2543593c 100644 --- a/spec/lib/cell_spec.rb +++ b/spec/lib/cell_spec.rb @@ -214,6 +214,104 @@ @cell.change_border(:diagonal, 'thin') expect(@cell.get_border(:diagonal)).to eq('thin') end + + context "when the cell is merged horizontally" do + before :each do + @worksheet.merge_cells(0, 0, 0, 1) + end + + it "should change the left border of both merged cells" do + @cell.change_border(:left, "thin") + expect(@worksheet[0][0].get_border(:left)).to eq("thin") + expect(@worksheet[0][1].get_border(:left)).to eq("thin") + end + + it "should change the right border of the both merged cells" do + @cell.change_border(:right, "thin") + expect(@worksheet[0][0].get_border(:right)).to eq("thin") + expect(@worksheet[0][1].get_border(:right)).to eq("thin") + end + + it "should change the top border for both merged cells" do + @cell.change_border(:top, "thin") + expect(@worksheet[0][0].get_border(:top)).to eq("thin") + expect(@worksheet[0][1].get_border(:top)).to eq("thin") + end + + it "should change the bottom border for both merged cells" do + @cell.change_border(:bottom, "thin") + expect(@worksheet[0][0].get_border(:bottom)).to eq("thin") + expect(@worksheet[0][1].get_border(:bottom)).to eq("thin") + end + end + + context "when the cell is merged vertically" do + before :each do + @worksheet.merge_cells(0, 0, 1, 0) + end + + it "should change the left border of both merged cells" do + @cell.change_border(:left, "thin") + expect(@worksheet[0][0].get_border(:left)).to eq("thin") + expect(@worksheet[1][0].get_border(:left)).to eq("thin") + end + + it "should change the right border of the both merged cells" do + @cell.change_border(:right, "thin") + expect(@worksheet[0][0].get_border(:right)).to eq("thin") + expect(@worksheet[1][0].get_border(:right)).to eq("thin") + end + + it "should change the top border for both merged cells" do + @cell.change_border(:top, "thin") + expect(@worksheet[0][0].get_border(:top)).to eq("thin") + expect(@worksheet[1][0].get_border(:top)).to eq("thin") + end + + it "should change the bottom border for both merged cells" do + @cell.change_border(:bottom, "thin") + expect(@worksheet[0][0].get_border(:bottom)).to eq("thin") + expect(@worksheet[1][0].get_border(:bottom)).to eq("thin") + end + end + + context "when the cell is merged horizontally and veritically" do + before :each do + @worksheet.merge_cells(0, 0, 1, 1) + end + + it "should change the left border of both merged cells" do + @cell.change_border(:left, "thin") + expect(@worksheet[0][0].get_border(:left)).to eq("thin") + expect(@worksheet[0][1].get_border(:left)).to eq("thin") + expect(@worksheet[1][0].get_border(:left)).to eq("thin") + expect(@worksheet[1][1].get_border(:left)).to eq("thin") + end + + it "should change the right border of the both merged cells" do + @cell.change_border(:right, "thin") + expect(@worksheet[0][0].get_border(:right)).to eq("thin") + expect(@worksheet[0][1].get_border(:right)).to eq("thin") + expect(@worksheet[1][0].get_border(:right)).to eq("thin") + expect(@worksheet[1][1].get_border(:right)).to eq("thin") + end + + it "should change the top border for both merged cells" do + @cell.change_border(:top, "thin") + expect(@worksheet[0][0].get_border(:top)).to eq("thin") + expect(@worksheet[0][1].get_border(:top)).to eq("thin") + expect(@worksheet[1][0].get_border(:top)).to eq("thin") + expect(@worksheet[1][1].get_border(:top)).to eq("thin") + end + + it "should change the bottom border for both merged cells" do + @cell.change_border(:bottom, "thin") + expect(@worksheet[0][0].get_border(:bottom)).to eq("thin") + expect(@worksheet[0][1].get_border(:bottom)).to eq("thin") + expect(@worksheet[1][0].get_border(:bottom)).to eq("thin") + expect(@worksheet[1][1].get_border(:bottom)).to eq("thin") + end + end end describe '.value' do From f7e57543345ea0689d6ccfbb0185af61d11d0d69 Mon Sep 17 00:00:00 2001 From: Michael Dotterer Date: Thu, 17 Aug 2017 11:41:01 -0400 Subject: [PATCH 2/3] Prevent changing column/row border from bleeding over merged cells --- lib/rubyXL/convenience_methods.rb | 17 ++++++--------- spec/lib/worksheet_spec.rb | 36 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/lib/rubyXL/convenience_methods.rb b/lib/rubyXL/convenience_methods.rb index cd9dd7a0b..d10b3c542 100644 --- a/lib/rubyXL/convenience_methods.rb +++ b/lib/rubyXL/convenience_methods.rb @@ -577,7 +577,7 @@ def change_row_border(row, direction, weight) sheet_data.rows[row].style_index = @workbook.modify_border(get_row_style(row), direction, weight) sheet_data[row].cells.each { |c| - c.change_border(direction, weight) unless c.nil? + c.change_border(direction, weight, :direct_match) unless c.nil? } end @@ -753,10 +753,9 @@ def change_column_border(column_index, direction, weight) ensure_cell_exists(0, column_index) cols.get_range(column_index).style_index = @workbook.modify_border(get_col_style(column_index), direction, weight) - sheet_data.rows.each { |row| c = row.cells[column_index] - c.change_border(direction, weight) unless c.nil? + c.change_border(direction, weight, :direct_match) unless c.nil? } end @@ -856,21 +855,17 @@ def change_text_wrap(wrap = false) self.style_index = workbook.modify_alignment(self.style_index) { |a| a.wrap_text = wrap } end - def change_border(direction, weight) + def change_border(direction, weight, direct_match = false) validate_worksheet - merged_cell = worksheet.merged_cells && worksheet.merged_cells.detect { |mc| mc.cover?(row, column) } - cells = [] + merged_cell = !direct_match && worksheet.merged_cells && worksheet.merged_cells.detect { |mc| mc.cover?(row, column) } if merged_cell merged_cell.ref.row_range.each do |r| merged_cell.ref.col_range.each do |c| - cells << worksheet.add_cell(r, c, '', nil, false) + worksheet.add_cell(r, c, '', nil, false).change_border(direction, weight, :direct_match) end end else - cells << self - end - cells.each do |cell| - cell.style_index = workbook.modify_border(cell.style_index, direction, weight) + self.style_index = workbook.modify_border(self.style_index, direction, weight) end end diff --git a/spec/lib/worksheet_spec.rb b/spec/lib/worksheet_spec.rb index 51345d71c..3ab355ea3 100644 --- a/spec/lib/worksheet_spec.rb +++ b/spec/lib/worksheet_spec.rb @@ -325,6 +325,24 @@ expect(@worksheet.get_row_border(0, :diagonal)).to eq('thin') expect(@worksheet[0][5].get_border(:diagonal)).to eq('thin') end + + context "with merged cells" do + before :each do + @worksheet.merge_cells(0, 2, 1, 2) + end + + it "should not add border outside of a merged cell when the border goes across it" do + @worksheet.change_row_border(0, :bottom, 'thin') + expect(@worksheet[0][0].get_border(:bottom)).to eq('thin') + expect(@worksheet[1][2].get_border(:bottom)).to be_nil + end + + it "should add border outside of a merged cell when the border is tangential to it" do + @worksheet.change_row_border(1, :bottom, 'thin') + expect(@worksheet[1][0].get_border(:bottom)).to eq('thin') + expect(@worksheet[1][2].get_border(:bottom)).to eq('thin') + end + end end describe '.change_column_font_name' do @@ -557,6 +575,24 @@ expect(@worksheet.get_column_border(0, :diagonal)).to eq('thin') expect(@worksheet[5][0].get_border(:diagonal)).to eq('thin') end + + context "with merged cells" do + before :each do + @worksheet.merge_cells(2, 0, 2, 1) + end + + it "should not add border outside of a merged cell when the border goes across it" do + @worksheet.change_column_border(0, :right, 'thin') + expect(@worksheet[0][0].get_border(:right)).to eq('thin') + expect(@worksheet[2][1].get_border(:right)).to be_nil + end + + it "should add border outside of a merged cell when the border is tangential to it" do + @worksheet.change_column_border(1, :bottom, 'thin') + expect(@worksheet[0][1].get_border(:bottom)).to eq('thin') + expect(@worksheet[2][1].get_border(:bottom)).to eq('thin') + end + end end describe '.merge_cells' do From 6ef108f5dc5b4f112b0f43f3401458995744bad6 Mon Sep 17 00:00:00 2001 From: Michael Dotterer Date: Thu, 17 Aug 2017 12:06:33 -0400 Subject: [PATCH 3/3] Apply border color changes intuitively to merged cells --- lib/rubyXL/convenience_methods.rb | 17 ++++- spec/lib/cell_spec.rb | 98 ++++++++++++++++++++++++ spec/lib/worksheet_spec.rb | 120 ++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 4 deletions(-) diff --git a/lib/rubyXL/convenience_methods.rb b/lib/rubyXL/convenience_methods.rb index d10b3c542..5ca4a7bab 100644 --- a/lib/rubyXL/convenience_methods.rb +++ b/lib/rubyXL/convenience_methods.rb @@ -589,7 +589,7 @@ def change_row_border_color(row, direction, color = '000000') sheet_data.rows[row].style_index = @workbook.modify_border_color(get_row_style(row), direction, color) sheet_data[row].cells.each { |c| - c.change_border_color(direction, color) unless c.nil? + c.change_border_color(direction, color, :direct_match) unless c.nil? } end @@ -768,7 +768,7 @@ def change_column_border_color(column_index, direction, color) sheet_data.rows.each { |row| c = row.cells[column_index] - c.change_border_color(direction, color) unless c.nil? + c.change_border_color(direction, color, :direct_match) unless c.nil? } end @@ -869,10 +869,19 @@ def change_border(direction, weight, direct_match = false) end end - def change_border_color(direction, color) + def change_border_color(direction, color, direct_match = false) validate_worksheet Color.validate_color(color) - self.style_index = workbook.modify_border_color(self.style_index, direction, color) + merged_cell = !direct_match && worksheet.merged_cells && worksheet.merged_cells.detect { |mc| mc.cover?(row, column) } + if merged_cell + merged_cell.ref.row_range.each do |r| + merged_cell.ref.col_range.each do |c| + worksheet.add_cell(r, c, '', nil, false).change_border_color(direction, color, :direct_match) + end + end + else + self.style_index = workbook.modify_border_color(self.style_index, direction, color) + end end def is_italicized() diff --git a/spec/lib/cell_spec.rb b/spec/lib/cell_spec.rb index f2543593c..42c7c45db 100644 --- a/spec/lib/cell_spec.rb +++ b/spec/lib/cell_spec.rb @@ -182,6 +182,104 @@ expect(@cell.get_border_color(:top)).to eq('FF0000') expect(@cell.get_border(:top)).to eq('thin') end + + context "when the cell is merged horizontally" do + before :each do + @worksheet.merge_cells(0, 0, 0, 1) + end + + it "should change the left border color of both merged cells" do + @cell.change_border_color(:left, "FF0000") + expect(@worksheet[0][0].get_border_color(:left)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:left)).to eq("FF0000") + end + + it "should change the right border color of the both merged cells" do + @cell.change_border_color(:right, "FF0000") + expect(@worksheet[0][0].get_border_color(:right)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:right)).to eq("FF0000") + end + + it "should change the top border color for both merged cells" do + @cell.change_border_color(:top, "FF0000") + expect(@worksheet[0][0].get_border_color(:top)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:top)).to eq("FF0000") + end + + it "should change the bottom border color for both merged cells" do + @cell.change_border_color(:bottom, "FF0000") + expect(@worksheet[0][0].get_border_color(:bottom)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:bottom)).to eq("FF0000") + end + end + + context "when the cell is merged vertically" do + before :each do + @worksheet.merge_cells(0, 0, 1, 0) + end + + it "should change the left border color of both merged cells" do + @cell.change_border_color(:left, "FF0000") + expect(@worksheet[0][0].get_border_color(:left)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:left)).to eq("FF0000") + end + + it "should change the right border color of the both merged cells" do + @cell.change_border_color(:right, "FF0000") + expect(@worksheet[0][0].get_border_color(:right)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:right)).to eq("FF0000") + end + + it "should change the top border color for both merged cells" do + @cell.change_border_color(:top, "FF0000") + expect(@worksheet[0][0].get_border_color(:top)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:top)).to eq("FF0000") + end + + it "should change the bottom border color for both merged cells" do + @cell.change_border_color(:bottom, "FF0000") + expect(@worksheet[0][0].get_border_color(:bottom)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:bottom)).to eq("FF0000") + end + end + + context "when the cell is merged horizontally and veritically" do + before :each do + @worksheet.merge_cells(0, 0, 1, 1) + end + + it "should change the left border color of both merged cells" do + @cell.change_border_color(:left, "FF0000") + expect(@worksheet[0][0].get_border_color(:left)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:left)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:left)).to eq("FF0000") + expect(@worksheet[1][1].get_border_color(:left)).to eq("FF0000") + end + + it "should change the right border color of the both merged cells" do + @cell.change_border_color(:right, "FF0000") + expect(@worksheet[0][0].get_border_color(:right)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:right)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:right)).to eq("FF0000") + expect(@worksheet[1][1].get_border_color(:right)).to eq("FF0000") + end + + it "should change the top border color for both merged cells" do + @cell.change_border_color(:top, "FF0000") + expect(@worksheet[0][0].get_border_color(:top)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:top)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:top)).to eq("FF0000") + expect(@worksheet[1][1].get_border_color(:top)).to eq("FF0000") + end + + it "should change the bottom border color for both merged cells" do + @cell.change_border_color(:bottom, "FF0000") + expect(@worksheet[0][0].get_border_color(:bottom)).to eq("FF0000") + expect(@worksheet[0][1].get_border_color(:bottom)).to eq("FF0000") + expect(@worksheet[1][0].get_border_color(:bottom)).to eq("FF0000") + expect(@worksheet[1][1].get_border_color(:bottom)).to eq("FF0000") + end + end end describe '.change_border' do diff --git a/spec/lib/worksheet_spec.rb b/spec/lib/worksheet_spec.rb index 3ab355ea3..7259b3432 100644 --- a/spec/lib/worksheet_spec.rb +++ b/spec/lib/worksheet_spec.rb @@ -345,6 +345,70 @@ end end + describe '.change_row_border_color' do + + it 'should cause error if a negative argument is passed in' do + expect { + @worksheet.change_row_border_color(-1, :left, 'FF0000') + }.to raise_error(RuntimeError) + end + + it 'should create a new row if it did not exist before' do + expect(@worksheet.sheet_data[11]).to be_nil + @worksheet.change_row_border_color(11, :left, 'FF0000') + expect(@worksheet.sheet_data[11]).to be_a(RubyXL::Row) + expect(@worksheet.get_row_border_color(11, :left)).to eq('FF0000') + end + + it 'should cause row and cells to have border color at top of specified weight' do + @worksheet.change_row_border_color(0, :top, 'FF0000') + expect(@worksheet.get_row_border_color(0, :top)).to eq('FF0000') + expect(@worksheet[0][5].get_border_color(:top)).to eq('FF0000') + end + + it 'should cause row and cells to have border color at left of specified weight' do + @worksheet.change_row_border_color(0, :left, 'FF0000') + expect(@worksheet.get_row_border_color(0, :left)).to eq('FF0000') + expect(@worksheet[0][5].get_border_color(:left)).to eq('FF0000') + end + + it 'should cause row and cells to have border color at right of specified weight' do + @worksheet.change_row_border_color(0, :right, 'FF0000') + expect(@worksheet.get_row_border_color(0, :right)).to eq('FF0000') + expect(@worksheet[0][5].get_border_color(:right)).to eq('FF0000') + end + + it 'should cause row to have border color at bottom of specified weight' do + @worksheet.change_row_border_color(0, :bottom, 'FF0000') + expect(@worksheet.get_row_border_color(0, :bottom)).to eq('FF0000') + expect(@worksheet[0][5].get_border_color(:bottom)).to eq('FF0000') + end + + it 'should cause row to have border color at diagonal of specified weight' do + @worksheet.change_row_border_color(0, :diagonal, 'FF0000') + expect(@worksheet.get_row_border_color(0, :diagonal)).to eq('FF0000') + expect(@worksheet[0][5].get_border_color(:diagonal)).to eq('FF0000') + end + + context "with merged cells" do + before :each do + @worksheet.merge_cells(0, 2, 1, 2) + end + + it "should not add border color outside of a merged cell when the border goes across it" do + @worksheet.change_row_border_color(0, :bottom, 'FF0000') + expect(@worksheet[0][0].get_border_color(:bottom)).to eq('FF0000') + expect(@worksheet[1][2].get_border_color(:bottom)).to be_nil + end + + it "should add border color outside of a merged cell when the border is tangential to it" do + @worksheet.change_row_border_color(1, :bottom, 'FF0000') + expect(@worksheet[1][0].get_border_color(:bottom)).to eq('FF0000') + expect(@worksheet[1][2].get_border_color(:bottom)).to eq('FF0000') + end + end + end + describe '.change_column_font_name' do it 'should cause column and cell font names to match string passed in' do @worksheet.change_column_font_name(0, 'Arial') @@ -595,6 +659,62 @@ end end + describe '.change_column_border_color' do + it 'should cause error if a negative argument is passed in' do + expect { + @worksheet.change_column_border_color(-1, :top, 'FF0000') + }.to raise_error(RuntimeError) + end + + it 'should cause column and cells wiFF0000 to have border color at top of specified weight' do + @worksheet.change_column_border_color(0, :top, 'FF0000') + expect(@worksheet.get_column_border_color(0, :top)).to eq('FF0000') + expect(@worksheet[5][0].get_border_color(:top)).to eq('FF0000') + end + + it 'should cause column and cells wiFF0000 to have border color at left of specified weight' do + @worksheet.change_column_border_color(0, :left, 'FF0000') + expect(@worksheet.get_column_border_color(0, :left)).to eq('FF0000') + expect(@worksheet[5][0].get_border_color(:left)).to eq('FF0000') + end + + it 'should cause column and cells wiFF0000 to have border color at right of specified weight' do + @worksheet.change_column_border_color(0, :right, 'FF0000') + expect(@worksheet.get_column_border_color(0, :right)).to eq('FF0000') + expect(@worksheet[5][0].get_border_color(:right)).to eq('FF0000') + end + + it 'should cause column and cells wiFF0000 to have border color at bottom of specified weight' do + @worksheet.change_column_border_color(0, :bottom, 'FF0000') + expect(@worksheet.get_column_border_color(0, :bottom)).to eq('FF0000') + expect(@worksheet[5][0].get_border_color(:bottom)).to eq('FF0000') + end + + it 'should cause column and cells wiFF0000 to have border color at diagonal of specified weight' do + @worksheet.change_column_border_color(0, :diagonal, 'FF0000') + expect(@worksheet.get_column_border_color(0, :diagonal)).to eq('FF0000') + expect(@worksheet[5][0].get_border_color(:diagonal)).to eq('FF0000') + end + + context "with merged cells" do + before :each do + @worksheet.merge_cells(2, 0, 2, 1) + end + + it "should not add border color outside of a merged cell when the border goes across it" do + @worksheet.change_column_border_color(0, :right, 'FF0000') + expect(@worksheet[0][0].get_border_color(:right)).to eq('FF0000') + expect(@worksheet[2][1].get_border_color(:right)).to be_nil + end + + it "should add border color outside of a merged cell when the border is tangential to it" do + @worksheet.change_column_border_color(1, :bottom, 'FF0000') + expect(@worksheet[0][1].get_border_color(:bottom)).to eq('FF0000') + expect(@worksheet[2][1].get_border_color(:bottom)).to eq('FF0000') + end + end + end + describe '.merge_cells' do it 'should merge cells in any valid range specified by indices' do @worksheet.merge_cells(0, 0, 1, 1)