D25423: Fill table cells with color for ODS and XLSX files

[Dag Andersen]

danders added a comment.


  ping?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: danders, dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Mikhail Philippov]

mphilippov added a comment.


  In D25423#589481 , @pvuorela wrote:
  
  > > Do you mean this "if ()" part of code? If there is no style "graphic" and 
inner "if" will not execute. Do you mean it'll be better to handle it in this 
part of code...or what?
  >
  > The main question was how should the three attributes play together. But 
another question: why isn't setBackgroundColor enough for the background color?
  
  
  Ok. 1. There is a bug in calligra with alpla-channel. So with 
style->setBackgroundColor(QColor()); all transparent cells will be painted with 
black color.
  
  2. This part of code isn't handled in all spreadsheet formats either ods or 
xls or xlsx. Setting brush needs us to paint all the cell with the required 
color

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Pekka Vuorela]

pvuorela added a comment.


  > Do you mean this "if ()" part of code? If there is no style "graphic" and 
inner "if" will not execute. Do you mean it'll be better to handle it in this 
part of code...or what?
  
  The main question was how should the three attributes play together. But 
another question: why isn't setBackgroundColor enough for the background color?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Mikhail Philippov]

mphilippov added a comment.


  In D25423#589430 , @pvuorela wrote:
  
  > > loadTableCellProperties() is a method for applying styles for table 
cells, i.e. cell background. There is no handling for cell background in the 
place you mentioned.
  >
  > I'm talking about handling in that particular method. It will already set 
background brush in that method later. Why is that not enough?
  
  
  Do you mean this "if ()" part of code? If there is no style "graphic" and 
inner "if" will not execute. Do you mean it'll be better to handle it in this 
part of code...or what?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Pekka Vuorela]

pvuorela added a comment.


  > loadTableCellProperties() is a method for applying styles for table cells, 
i.e. cell background. There is no handling for cell background in the place you 
mentioned.
  
  I'm talking about handling in that particular method. It will already set 
background brush in that method later. Why is that not enough?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Mikhail Philippov]

mphilippov added a comment.


  In D25423#586953 , @pvuorela wrote:
  
  > Not sure about all the details but two things look suspicious here:
  >
  > 1. Above this there's handling for transparent and now the handling isn't 
symmetric for background brush.
  > 2. There's some handling on the file for KoXmlNs::draw / style-name & 
KoXmlNs::calligra / fill-style-name which do background brush setting. What's 
the relation between these three?
  
  
  
  
  1. I see it. Ok
  2. loadTableCellProperties() is a method for applying styles for table cells, 
i.e. cell background. There is no handling for cell background in the place you 
mentioned.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Pekka Vuorela]

pvuorela added a comment.


  Not sure about all the details but two things look suspicious here:
  
  1. Above this there's handling for transparent and now the handling isn't 
symmetric for background brush.
  2. There's some handling on the file for KoXmlNs::draw / style-name & 
KoXmlNs::calligra / fill-style-name which do background brush setting. What's 
the relation between these three?

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever

D25423: Fill table cells with color for ODS and XLSX files

[Mikhail Philippov]

mphilippov created this revision.
mphilippov added reviewers: denexter, pvuorela, davidllewellynjones, dcaliste.
mphilippov added a project: Calligra: 3.0.
Herald added a subscriber: Calligra-Devel-list.
mphilippov requested review of this revision.

REVISION SUMMARY
  Even when the tables in XLS/XLSX files are painted with some color, this 
color isn't displayed.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25423

AFFECTED FILES
  sheets/odf/SheetsOdfStyle.cpp

To: mphilippov, denexter, pvuorela, davidllewellynjones, dcaliste
Cc: dcaliste, davidllewellynjones, denexter, Calligra-Devel-list, 
#calligra:_3.0, ognarb, cochise, vandenoever