MINOR: Fix theme extends to respect multiple image textures #2230
Conversation
germanz
force-pushed
the
themeExtendsImages
branch
from
a9c787f
to
184a784
Compare
Codecov Report
@@ Coverage Diff @@
## master #2230 +/- ##
==========================================
+ Coverage 67.29% 67.30% +0.01%
==========================================
Files 312 312
Lines 27696 27709 +13
Branches 6184 6188 +4
==========================================
+ Hits 18637 18650 +13
Misses 9059 9059
Continue to review full report at Codecov.
|
| const result = await ThemeLoader.load(inheritedTheme); | ||
| assert.exists(result.images?.foo); | ||
| assert.exists(result.images?.bar); | ||
| assert.deepEqual(result.imageTextures?.map(imageTexture => imageTexture.name).sort(), [ |
There was a problem hiding this comment.
Please change one of the imageTextures to have a collision and check that the second one overrides the first one.
There was a problem hiding this comment.
Sorry, didn't mean to resolve.
It is better, but if I understand correctly, the test still doesn't test that the collision is resolved correctly because the "baz" points to the same image as the base theme. So you can't say for certain that you have the ImageTexture from the theme.
If you change the image:"baz" to something different (but keep the "name" the same), then you can be sure that the collision is correctly resolved.
There was a problem hiding this comment.
Nevermind, I misunderstood the code. Resolving and will approve.
Signed-off-by: German Zargaryan <2526045+germanz@users.noreply.github.com>
germanz
force-pushed
the
themeExtendsImages
branch
from
184a784
to
84c4504
Compare
The specified allows theme extends to work properly when multiple images (textures) are specified.