From 7257e9abcd9f576deb08d040699189d48b297b21 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Fri, 30 Apr 2021 18:43:59 +0200 Subject: [PATCH 1/3] Pattern Fill style should default to 'solid' if there is a pattern fill style for a conditional; though may need to check if there are defined fg/bg colours as well; and only set a fill style if there are defined colurs --- src/PhpSpreadsheet/Reader/Xlsx/Styles.php | 2 +- src/PhpSpreadsheet/Writer/Xls/Style/ColorMap.php | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php index 2b0c7016f3..4f2723e7bd 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php @@ -105,7 +105,7 @@ public static function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyl } elseif ($fillStyleXml->patternFill) { $patternType = (string) $fillStyleXml->patternFill['patternType'] != '' ? (string) $fillStyleXml->patternFill['patternType'] - : Fill::FILL_NONE; + : Fill::FILL_SOLID; $fillStyle->setFillType($patternType); if ($fillStyleXml->patternFill->fgColor) { diff --git a/src/PhpSpreadsheet/Writer/Xls/Style/ColorMap.php b/src/PhpSpreadsheet/Writer/Xls/Style/ColorMap.php index e3a6b2063b..caf85c0499 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Style/ColorMap.php +++ b/src/PhpSpreadsheet/Writer/Xls/Style/ColorMap.php @@ -75,6 +75,16 @@ public static function lookup(Color $color, int $defaultIndex = 0x00): int return self::$colorMap["#{$colorRgb}"]; } +// TODO Try and map RGB value to nearest colour within the define pallette +// $red = Color::getRed($colorRgb, false); +// $green = Color::getGreen($colorRgb, false); +// $blue = Color::getBlue($colorRgb, false); + +// $paletteSpace = 3; +// $newColor = ($red * $paletteSpace / 256) * ($paletteSpace * $paletteSpace) + +// ($green * $paletteSpace / 256) * $paletteSpace + +// ($blue * $paletteSpace / 256); + return $defaultIndex; } } From 6eff7a592c7061e67eff8c15d8747e5869393b32 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Fri, 30 Apr 2021 19:33:52 +0200 Subject: [PATCH 2/3] Provide a unit test --- src/PhpSpreadsheet/Reader/Xlsx/Styles.php | 14 +++++++++----- .../Reader/Xlsx/DefaultFillTest.php | 11 +++++++++++ tests/data/Reader/XLSX/pr2050cf-fill.xlsx | Bin 0 -> 8267 bytes 3 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 tests/data/Reader/XLSX/pr2050cf-fill.xlsx diff --git a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php index 4f2723e7bd..2968a3feaf 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php @@ -103,17 +103,21 @@ public static function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyl self::readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=1]'))->color) ); } elseif ($fillStyleXml->patternFill) { - $patternType = (string) $fillStyleXml->patternFill['patternType'] != '' - ? (string) $fillStyleXml->patternFill['patternType'] - : Fill::FILL_SOLID; - - $fillStyle->setFillType($patternType); + $defaultFillStyle = Fill::FILL_NONE; if ($fillStyleXml->patternFill->fgColor) { $fillStyle->getStartColor()->setARGB(self::readColor($fillStyleXml->patternFill->fgColor, true)); + $defaultFillStyle = Fill::FILL_SOLID; } if ($fillStyleXml->patternFill->bgColor) { $fillStyle->getEndColor()->setARGB(self::readColor($fillStyleXml->patternFill->bgColor, true)); + $defaultFillStyle = Fill::FILL_SOLID; } + + $patternType = (string) $fillStyleXml->patternFill['patternType'] != '' + ? (string) $fillStyleXml->patternFill['patternType'] + : $defaultFillStyle; + + $fillStyle->setFillType($patternType); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php index 88d666b212..ccdad06795 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php @@ -29,4 +29,15 @@ public function testDefaultFill(): void self::assertSame('none', $sheet->getCell('J16')->getStyle()->getFill()->getFillType()); self::assertSame('solid', $sheet->getCell('C2')->getStyle()->getFill()->getFillType()); } + + public function testDefaultConditionalFill(): void + { + // default fill pattern for a conditional style where the filltype is not defined + $filename = 'tests/data/Reader/XLSX/pr2050cf-fill.xlsx'; + $reader = IOFactory::createReader('Xlsx'); + $spreadsheet = $reader->load($filename); + + $style = $spreadsheet->getActiveSheet()->getConditionalStyles('A1')[0]->getStyle(); + self::assertSame('solid', $style->getFill()->getFillType()); + } } diff --git a/tests/data/Reader/XLSX/pr2050cf-fill.xlsx b/tests/data/Reader/XLSX/pr2050cf-fill.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..e2183422d25a46b3513812937cb19a0c73c6a562 GIT binary patch literal 8267 zcmeHMg;$hY`yCn-kS;+$8l<~B1cafK8oGz>9sy~P8cIS^kQ|VPkuDLDk{m#g4(U?h zH~QWC-Fvo}T>&~OYaRd<`TKvb|KS;^NEy=T<{?l#Q~56c zg);_GD}yb#8`@9Q!b)ql{Lo^O+i!mCT9WvzMkb%sLAb_ZWZpxPKli=RClYW}DZG3- zBS9af>QuG0!Qy3X<6#nMMv=~#FBa(>YVFMNsZH%PW=Ci(&EpC+HVK>%FRaJ+&rqmy z1f&czH-sDR5BReg5u6o{hLytAmb5ed@q}{PWEh?>lUCoIT@Z~nOC%j>?a0;Rr3@Xd zSO210k#niYc80EzDT*mhv;W+YhgW4o^BQyKNUBsoi-UEI@0E}JV8k@V!S2f8&^IP& zPQIlv;rJ@q3xDg^sMPZ4uVH8^pZ43_IqJpFr8-ACSr7T%)+SvC8=S`_C|qGCwWk{m z@Yz1iXu#Z`wH_%b8JMW{J4Ug0XZZdym||F}6_*j6*IopF9hj1!@ZvJI`u0_;b{^*R z{q~j3J4goI+@Jw~f01Fm9uLDY@`*Z9Ww((sGR9k$`E+)_jhv$iTJGn z`qO2BidZ5NafSw;%82w!H!m!9X7@BDw~Eg_gx-@Ulc$-=s(!5A@C5eqrqX=1!9@n8 z@dJe#f)TDKR5--N)DdKonIVRKj|>(p&Z|%+<#cu{BVdifd0QzXS&*r;SDSd!QKITQ zW0|A_9u~GURe=MJ^d}codOCKZ_ArZFcd`3^rdEy}M+#Z(#8-iw8rcKd_XUW~`9_rc znet8`hIIl5Lpgpu!Z_XK1N(!av9zTNNJsj+NHRJdidRce#^t(n44uK4l9uZOHn2?^+)LVrzYS&P`jtQYh1!XruQ$5SX`>+=DQv@adY$69Q)-LzPusLicv#E3uL;*dA?Xrxn=X%<_h7 zC@r?rxIAkE-Esq?$W#*P+97(L7uJUA>C!|;OcEI27--?iH+$l&6_lmf( zzXR7PN3AOJ^f>hLr^QrM>Q|o8C8z451?lZ8o!IG}5;n!Wn`@}y*AfKOd|{eu#V0Y) zBYRl|rr)eZpD|iCGQxQRfQ06|eaoafb}oyt)n=_3QwHqrJZvA=gvk$-lQ=G}FIlSO zGR#N{3rfI}J6Q}cuqu`)^Mmc!yP24V;_1tVC=(j!B|8vmgaUy7VZ-J%EEJ7Lqk`{ zi|c2}#vcP%38v{FhV3Ol6uzd{{FEtKk$zO~3k9fWrSNSUgzw_+xDi}>^U!}zG^4%@ z+aT;2p!KRzOKmGvq?lB&}ty>;2KFc)MI z@#*x%RP!_=>tP^4{dS~0tw9!-=dcIpX;0L%NLy(N61?-4ynzbrO)3){e+muTO~I9M=*{ z{A|5TZXOg`V?GxKomr^B?otjrmP6PPCuTzU$5D~}^3;Apxp;wdw;87J;UQp6_(aZy z{F2ncBd_=N1UCtiHAjK-wEPj)vc+$q0?W-bb$l+(66Mh|*5hUq-s~9>k z1C?v&y#mlD(DtM~vq}C0G5s8hkWmqu!LGtYcrZHoNU!8+kTjoZ$S1Al4H%f7RZZ^e z1xdg|EqJSh!hDL%??-?x&S6(Yoho8IAX@!mvVJPDr2UA2hQd+THMC{{`#|dWA_^ni zzI*CwO@l||aW~?565G1p4CUviM)Jzo7!P-y7X{$OJ}wCiqRmfb((F~pfvM4)(Gl(0 zOv)WY=fjc*$+_W=FR9&3V!`;mk~D9FBwUpl%lm74MKsAMCBA(*B>yLVuof$F-9d6g z8d(5PAQ%3(P1VEB+S=2D`{%&>V|UF?(fYBwhA*=0O6v#T!BkCVMlEAxHKb*J1Zwo@ zH|~h3$=nr)esy&Ql6lBteMgH)rlCgqrRB>ksT@Pag$^-$Po+Y-BJ?_*;t4M$NSiZD z0#szgYcO>*ag<-=5(>G7-it@HVs;QS+NF>7eRCenAjYek$9RPn4`H`7+N%MB*TinO zQIplj^qPxUa;Cp{{;U`tX5^6nGGJ$h~%sVTJI0rmp1%SsgkweoCAn*n@s>%wDjR%b-UA zdM6cV950fPDnDNxYm==7rzRnQ?C~g}LjC*V(n=gil*BmVdnfR^YV01IVWt2rU;DzU z@uqyqf=FpdM!KP(7i@Ps?RMI!4(nC~16Lc;W(hGS&CE2SX=ox)8sMA$MjoE6{0mv9 zV_+B%#rheF3CJ2(l~Vjl)>OwOwvw%DUIvl^IdQyfoWlB#4!cIXq5w{u5Z}Udd$z4+;*23l}YqOo3*GoUCu=k zIwpgL2R}~0VXWJv9c3AwW~f|c%yLvU^u~)-zJ`~SoaF}lA9)^(!881oGj4Y!r}xP;ksoqkSkk$4=IZ z9g)=3VIvZ<$Q;+N(#7zSte=`DY$1b;qgcC=f+6|bB-8>J5X48%u!EPX3JG}KrumsS zNe(ZLb@54A4O4#2N_VYXRQQJW9u58Mp?NN^_`T(Tu`^a8qF}oVrmJ=GMsBjv$IJ?= zmSlrZqaKHwoepDk*kOhN#inSb#{DPTSBC8kVp|8m7gzQ=Jpd z@qm|;`K?dB>J5=Cd^82UX{ISl2%pN+g}@?caa)!YZ&%B*Nf~!X=Gc_jZcpWrm>P9Y zsqLBo8;`+2g@^7<5nSQ*4nEDZq*h%U&xsds!(`#fY$IBUc;!3VI_moOg;x#Z_V}a# zPf@iQA1)8};%a`xu5F5C6ez{6LR3Nnk< zzpH)fYtb{P-te6xZ&H|!J6AH`dVljMvRx8#ym8TOTH9RDuI7xzq@%MpnP7Tzbw$2l z+J3#ibs)Ss!E(^m)BgQpD82pWKa*on=2-I({BH*+(m6dmay`j1gd5(adqV@LCPs8yU0$%=t~qM`K8!YrcJNFK?Ljl+ z^UP|c8jN|%IByCcv8A#XL;L%#x$20Nqvq(==#NR@JucM>z}Y%~-jikRnRuA>8eR-h z^ed6hF`f1r`8b;LR{5yaDM`L%by)QA9zk_j)Hsv8yBU)uq_6^x_Y3*o>D)0)@yp6T`!sia@RA#Q?W>oNxVLQ6CeDF+d zO)T<+O1AwL?h5|5d!QbnWHtAVvF2C`{v=87>3E=o$E0{^< zx`Rfd^cYT@@+H1^B)XR6yU)9OJ}4fT7zsuP-=Jbytk&xaA#ZTbXdi(6?Q-WkT$3w2 zKKbc+m`tAEd@HjVH<7Bg{s7KAGVD7!J&|?DYzqu|$NcqhBUiJnW|f~Fx;KQ;+bZW1 zgx)q9hs9Gmr`5vJ?11v&{`m{Rj0f{j2F?{XW@#3_*@Ta{hVs^kj_erq6J1|^<~+-y zNG}f3mme#?X&m_`#E)y(HdnN~czqiq`NEgYI9@(o_0@)<uPwy%2Gwv(J6~yVx>td@n(>Jy z{gAdg5fqEo$;0{bJ>MgPHjVC>H*d7&>i8dE<7!R_ojZ9ciAWw)u}BZ(yf^fcd9baT zbAI`5zHQP5%O^y_-xVTKqP+o>b4h1ly&Stc02MNeolh}{$6jm`W#PODW^=?ss7`Z2 zsm$96bi=gs2^-48y(u0KUBQgEyx8w7nI&l%Lppx;TnY>cRq=Qn^YQo#q&qNMyVzC z;S6~Avy;xiZQ)z>?u|y+4oud;g$AWqsjA|ojd!o|uIM-{%~sihtlk3799kQ0{;@D7 zpoMCVB6GL_WMPc=AN%$2^mDZK_>s63{ZSU1UF1;N4J2cirrjx0MF_mkse6JJp?{)m zl^y3e3t1FQd{#OxRL^Z@T9q&YU!U&rsU+FxQ17l+b#+~_?9t6vtJ*?`1ifpcREjSy zE0W_#TVeq3gD$T@^V5Z92}XSnP_ZW&eZjlVvMM@`nMK3#3%nSn>`sjJJc5Gu-Vvj< zZm~sl`Cm1oi;O?tGpCQeK}njB`08Vlzxv!T*Q0Z{rn^gpEcTnBy}SpC?wpz2?FmdB z;(|}vLYoELr%7!*Eu*P$6yt*O81fw^glHzQp>ZPM!b?R~;ro8s>8Ca#dpTTjR~A~0 z6;+P8Jh`u_7`!2+;Lc@(&9CepFO6X%sw-<)VnCE6K#BlY-A7D=M0~L+MUvEt5q+&r zk8Y5mA`^MP6m^SC$!Q{s=zDKczM6yPVzL%T+0Zwg0>uW zO1gP;I#mPDt4km-f%wDKq4d^XQ^ z6R&&JX;w2^-45fNl$5=hW&9YM4epwG=7^B5jG|MRSKJ1QXI)Y`(#+?4*FCCl(?5%D z-T3wFK67F({Uzk`=SwP5Gs8wznwqg~s6MCQ*A5j|pVd~mon>b|R(GU*Qg zuok>g9UNbkBWx^&;LPlE%_fh3T0V%|i_auwVu5q-k^d#=PRMD-H;L>^+QN>rS|jOw zW*xC3oAJ4Up#nN%O_rfYP(_BlQ4VgFm*}#PVbR0UF84w7CoGj3x8e)wh}3Vi(M8#0 zx!UuNg^|qudwp8~>PVD9s=^e>e?nw^Yvp1Iba!#};I?#exBl(;;@`3sIdyT#T58=t z3OI}#`N4p!x>R2IxsZ3c)Yuv%i*oLb7+C_0qt4g<3Z4qLr0x%wR0cd< zqHqA_!Mp8~m)}f<>Ftd~m}H8i9%B)Ei}mHyV$k-C!hPHY8l1A~{Ad}zXdcXM9WNtm7^gXWDj$ONZOX9WHS$XDBx$9Oux|@&499OZ-cK00>0<9?d$bFVo z!FGr~w^nM>gZRNm*@&vaxV2|%M`zB5)z@4LKw$sH4a+O&7vg0cAnEzCny&Oe($4%L z4nZ`Mc7(`&B*C9)XYT6yFYAy~_S==6EbB7IgB$)8?UEK3aaR-p#0{yiPD;DxqO?V#a?f{!4)GrNTo5wV3H~;%F~0dbNNA1N| z#Yo1fKj4*Ndj51-$uv+qi<;QV{V9erdWC`-^s#nrr=!SdR+G*>!o!HM)u64qBedYZI&m4!6t`))q!xD}I)(6%=b=V_NEZ$vE|OV5L0 zOR3qhF`Er$rnQBIT&dZO4&eAmL9FdQUv#EL%&mLxVRi9tl=WNolVQafw>=@gPEQKX zod__qRgam)iKA*T=O*InHB}6xd%wqV0`ij`Ga@WS&pv?96uFhr$k+Sm%}~uhQ@gDX zpj|r%F^`M)42d2L@EmA}%!Rm!d3+Ey$<(}L37~1U>PiaNIq43!4OD6M1!J2``AO-{ zeX=UOK-!NK-h`wJ-#2XO2kHq!Lareogzg95U1<014Y4N)D`o9G`3Mwby z|2z2oeYt)g|KS)IsP@+Ye?7AL9r$C6L~7zs2Ux!Xe{JLc0j)shFTXVPe+B=wo%sh8 z0Qija6a4=)H-C-uYuoFONGQ1feTly{!G4YMYv1FKC`$N0qx{+h`8B|=ncE)$dP#o< z_#=<|75Z1>`vV97bRzqBf3v|~;eXx0{tOqn`zQEs+Zj*|1GzQ;01omag>>38njhc( E55p);2><{9 literal 0 HcmV?d00001 From a61366550044453a6de302344ba1d15f76f05d9e Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Fri, 30 Apr 2021 19:55:02 +0200 Subject: [PATCH 3/3] Provide a unit test --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21762f893b..ed4f44c3f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Nothing. ### Fixed - +- Correct default fill style for conditional without a pattern defined [Issue #2035](https://github.com/PHPOffice/PhpSpreadsheet/issues/2035) [PR #2050](https://github.com/PHPOffice/PhpSpreadsheet/pull/2050) - Fixed issue where array key check for existince before accessing arrays in Xlsx.php. [PR #1970](https://github.com/PHPOffice/PhpSpreadsheet/pull/1970) - Fixed issue with quoted strings in number format mask rendered with toFormattedString() [Issue 1972#](https://github.com/PHPOffice/PhpSpreadsheet/issues/1972) [PR #1978](https://github.com/PHPOffice/PhpSpreadsheet/pull/1978) - Fixed issue with percentage formats in number format mask rendered with toFormattedString() [Issue 1929#](https://github.com/PHPOffice/PhpSpreadsheet/issues/1929) [PR #1928](https://github.com/PHPOffice/PhpSpreadsheet/pull/1928)