Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor code of theme/flavors, theme_none() uses blue colors #891

Merged
merged 8 commits into from
Sep 29, 2023
Prev Previous commit
Next Next commit
Use blue colors for 'none' (no symbolic colors), not apply flavor to it.
  • Loading branch information
OLarionova-HORIS committed Sep 29, 2023
commit f4f400a9a37e43aa925b9fe9949c7d1bf30338b8
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal class DefaultGeomTheme private constructor(
private const val TEXT_SIZE = 7.0

// defaults for geomKind
fun forGeomKind(geomKind: GeomKind, colorTheme: ColorTheme): GeomTheme {
internal fun forGeomKind(geomKind: GeomKind, colorTheme: ColorTheme): GeomTheme {

// Size: point size or line width - depending on the geom kind.
val size = when (geomKind) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ internal class ThemeFlavor private constructor(
ThemeOption.TOOLTIP_RECT to mapOf(
Elem.COLOR to DARK_GREY,
Elem.FILL to Color.WHITE
),
ThemeOption.RECT to mapOf(
Elem.FILL to LIGHT_GREY
),
)
),
pen = DARK_GREY,
brush = Color.PACIFIC_BLUE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,29 @@ object ThemeUtil {
themeName: String,
userOptions: Map<String, Any> = emptyMap(),
fontFamilyRegistry: FontFamilyRegistry = DefaultFontFamilyRegistry()
): DefaultTheme {
) = DefaultTheme(
getThemeValues(themeName, userOptions),
fontFamilyRegistry
)

// open for ThemeOptionTest
internal fun getThemeValues(themeName: String, userOptions: Map<String, Any> = emptyMap()): Map<String, Any> {
val baselineValues = ThemeValues.forName(themeName).values

val flavorName = userOptions[ThemeOption.FLAVOR] as? String
?: baselineValues[ThemeOption.FLAVOR] as? String
?: error("Flavor name should be specified")

val effectiveOptions = applyFlavor(baselineValues, flavorName).mergeWith(userOptions)
val flavorName = if (themeName != ThemeOption.Name.LP_NONE) {
// use flavor to not 'none' theme only
userOptions[ThemeOption.FLAVOR] as? String
?: baselineValues[ThemeOption.FLAVOR] as? String
?: error("Flavor name should be specified")
} else {
null
}

return DefaultTheme(effectiveOptions, fontFamilyRegistry)
return (flavorName?.let { applyFlavor(baselineValues, flavorName) } ?: baselineValues)
.mergeWith(userOptions)
}

internal fun applyFlavor(themeSettings: Map<String, Any>, flavorName: String): Map<String, Any> {
private fun applyFlavor(themeSettings: Map<String, Any>, flavorName: String): Map<String, Any> {
val flavor = ThemeFlavor.forName(flavorName)

val geomThemeOptions = mapOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.AXIS_TITLE
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.ELEMENT_BLANK
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.Elem
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.FLAVOR
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.Flavor
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.LEGEND_BKGR_RECT
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.LEGEND_DIRECTION
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.LEGEND_JUSTIFICATION
Expand Down Expand Up @@ -134,8 +132,6 @@ internal open class ThemeValuesBase : ThemeValues(VALUES) {
TOOLTIP_TITLE_TEXT to mapOf(
Elem.FONT_FACE to FontFace.BOLD,
),

FLAVOR to Flavor.BASE,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,53 @@

package org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values

internal class ThemeValuesLPNone : ThemeValuesBase()
import org.jetbrains.letsPlot.commons.values.Color
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.Elem
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.Geom
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.LEGEND_BKGR_RECT
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.LINE
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.PLOT_BKGR_RECT
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.RECT
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.TEXT
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.TOOLTIP_RECT

internal class ThemeValuesLPNone : ThemeValues(VALUES) {

companion object {

private val VALUES: Map<String, Any> = ThemeValuesBase() + mapOf(
LINE to mapOf(
Elem.COLOR to Color.BLUE,
),
RECT to mapOf(
Elem.COLOR to Color.BLUE,
Elem.FILL to Color.LIGHT_BLUE,
),
TEXT to mapOf(
Elem.COLOR to Color.BLUE,
),

PLOT_BKGR_RECT to mapOf(
Elem.FILL to Color.WHITE,
),

LEGEND_BKGR_RECT to mapOf(
Elem.FILL to Color.WHITE,
),

// Legend
// Tooltip
TOOLTIP_RECT to mapOf(
Elem.FILL to Color.WHITE,
Elem.COLOR to Color.BLACK
),

ThemeOption.GEOM to mapOf(
Geom.PEN to Color.BLUE,
Geom.PAPER to Color.WHITE,
Geom.BRUSH to Color.PACIFIC_BLUE,
),
)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,20 @@ class ThemeColorOptionsTest(
test(
themeOptions = noneTheme,
expectedPlotBackground = Color.WHITE,
expectedYAxisColor = DARK_GREY,
expectedYAxisColor = Color.BLUE,
expectedPanelBackground = panelBackgroundRect(
expectedColor = DARK_GREY,
expectedFill = LIGHT_GREY
expectedColor = Color.BLUE,
expectedFill = Color.LIGHT_BLUE
)
),
// 'none' theme with flavor: panel background is equal to the plot background
// flavor is not applied to 'none' theme
test(
themeOptions = noneTheme + flavorOption,
expectedPlotBackground = Color.parseHex("#303030"),
expectedYAxisColor = Color.parseHex("#BBBBBB"),
expectedPlotBackground = Color.WHITE,
expectedYAxisColor = Color.BLUE,
expectedPanelBackground = panelBackgroundRect(
expectedColor = Color.parseHex("#BBBBBB"),
expectedFill = Color.parseHex("#303030")
expectedColor = Color.BLUE,
expectedFill = Color.LIGHT_BLUE
)
),
// The 'classic' theme: facet rect fill = plot background
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,14 @@ import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.ForTest.numericOptions
import org.jetbrains.letsPlot.core.plot.builder.defaultTheme.values.ThemeOption.ForTest.themeNames
import org.jetbrains.letsPlot.core.plot.builder.presentation.DefaultFontFamilyRegistry
import org.jetbrains.letsPlot.core.spec.getString
import kotlin.test.Test

internal class ThemeOptionTest {

@Test
fun checkElements() {
for (themeName in themeNames) {
val themeValues = themeValues(themeName, withFlavor = true)
val theme = DefaultTheme(themeValues)

val theme = ThemeUtil.buildTheme(themeName)
for (elem in elemWithColorAndSize + elemWithColorOnly + elemWithFill) {
val elemKey = accessKeyForOption(theme, elem)
checkElemProperty(themeName, accessKeyForOption(theme, elem, COLOR), COLOR)
Expand All @@ -49,9 +46,7 @@ internal class ThemeOptionTest {
@Test
fun checkOptions() {
for (themeName in themeNames) {
val themeValues = themeValues(themeName, withFlavor = false)
val theme = DefaultTheme(themeValues)

val theme = ThemeUtil.buildTheme(themeName)
for (option in numericOptions) {
val optionKey = accessKeyForOption(theme, option)
checkNumericOption(themeName, optionKey)
Expand All @@ -60,7 +55,7 @@ internal class ThemeOptionTest {
}

private fun checkElemProperty(theme: String, elemKey: List<String>, elemProperty: String) {
val themeValues = themeValues(theme, withFlavor = true)
val themeValues = ThemeUtil.getThemeValues(theme)
val access = object : ThemeValuesAccess(themeValues, DefaultFontFamilyRegistry()) {
fun check() {
when (elemProperty) {
Expand All @@ -79,7 +74,7 @@ internal class ThemeOptionTest {
}

private fun checkNumericOption(theme: String, optionKey: List<String>) {
val themeValues = themeValues(theme, withFlavor = false)
val themeValues = ThemeUtil.getThemeValues(theme)
val access = object : ThemeValuesAccess(themeValues, DefaultFontFamilyRegistry()) {
fun check() {
this.getNumber(optionKey)
Expand Down Expand Up @@ -153,14 +148,4 @@ internal class ThemeOptionTest {
else -> throw IllegalStateException("Unknown theme option: $option")
}
}

private fun themeValues(themeName: String, withFlavor: Boolean): Map<String, Any> {
return ThemeValues.forName(themeName).values.let { baseValues ->
if (withFlavor) {
val flavorName = baseValues.getString(ThemeOption.FLAVOR) ?: error("Flavor name should be specified")
ThemeUtil.applyFlavor(baseValues, flavorName)
} else
baseValues
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,24 @@ class ThemeConfigTest {

@Test
fun withThemeName() {
val spec = plotSpec(themeName = "none")
val spec = plotSpec(themeName = "classic")
val colors = transformToClientPlotConfig(spec).theme.colors()

assertEquals(Color.parseHex("#474747"), colors.pen())
assertEquals(Color.WHITE, colors.paper())
assertEquals(Color.PACIFIC_BLUE, colors.brush())
}

@Test
fun withThemeNone() {
val spec = plotSpec(themeName = "none")
val colors = transformToClientPlotConfig(spec).theme.colors()

assertEquals(Color.BLUE, colors.pen())
assertEquals(Color.WHITE, colors.paper())
assertEquals(Color.PACIFIC_BLUE, colors.brush())
}

@Test
fun withFlavor() {
val spec = plotSpec(flavorName = "darcula")
Expand Down