Make PaletteSelect use stylehseets instead of inline styles (bokeh#…

…14052) * Make PaletteSelect use stylesheets instead of inline styles * Make color2css() pass through CSS color strings * Disable maximum image size checks in PIL
abdulrahman305 · Sep 6, 2024 · 4f93515 · 4f93515
1 parent 5036057
commit 4f93515
Show file tree

Hide file tree

Showing 11 changed files with 115 additions and 75 deletions.
diff --git a/bokehjs/src/less/widgets/palette_select_item.less b/bokehjs/src/less/widgets/palette_select_item.less
@@ -8,6 +8,12 @@
   gap: 0.5em;
 }
 
+.bk-swatch {
+  width: 100px;
+  height: auto;
+  align-self: stretch;
+}
+
 .bk-item {
   --active-tool-highlight: #26aae1;
 

diff --git a/bokehjs/src/lib/core/util/color.ts b/bokehjs/src/lib/core/util/color.ts
@@ -59,12 +59,17 @@ function hex(v: uint8): string {
 }
 
 export function rgba2css([r, g, b, a]: RGBA): string {
-  return `rgba(${r}, ${g}, ${b}, ${a/255})`
+  const alpha = a == 255 ? "" : ` / ${a/255}`
+  return `rgb(${r} ${g} ${b}${alpha})`
 }
 
 export function color2css(color: Color | null, alpha?: number): string {
-  const [r, g, b, a] = color2rgba(color, alpha)
-  return rgba2css([r, g, b, a])
+  if (isString(color) && (alpha == null || alpha == 1.0)) {
+    return color // passthrough to persist color in its original form
+  } else {
+    const [r, g, b, a] = color2rgba(color, alpha)
+    return rgba2css([r, g, b, a])
+  }
 }
 
 export function color2hex(color: Color | null, alpha?: number): string {

diff --git a/bokehjs/src/lib/models/widgets/palette_select.ts b/bokehjs/src/lib/models/widgets/palette_select.ts
@@ -1,12 +1,12 @@
-import type {Keys} from "core/dom"
-import {div, canvas, empty, px} from "core/dom"
-import type {StyleSheetLike} from "core/dom"
+import {div, empty, px, InlineStyleSheet} from "core/dom"
+import type {StyleSheetLike, Keys} from "core/dom"
 import {DropPane} from "core/util/panes"
 import type * as p from "core/properties"
 import {enumerate} from "core/util/iterator"
 import {color2css} from "core/util/color"
 import {cycle} from "core/util/math"
 import {linspace} from "core/util/array"
+import {assert} from "core/util/assert"
 
 import {InputWidget, InputWidgetView} from "./input_widget"
 import * as inputs_css from "styles/widgets/inputs.css"
@@ -28,8 +28,11 @@ export class PaletteSelectView extends InputWidgetView {
   protected _value_el: HTMLElement
   protected _pane: DropPane
 
+  protected readonly _style = new InlineStyleSheet()
+  protected readonly _style_menu = new InlineStyleSheet()
+
   override stylesheets(): StyleSheetLike[] {
-    return [...super.stylesheets(), palette_select_css.default, item_css.default, icons_css.default]
+    return [...super.stylesheets(), palette_select_css.default, item_css.default, icons_css.default, this._style]
   }
 
   override connect_signals(): void {
@@ -53,56 +56,12 @@ export class PaletteSelectView extends InputWidgetView {
     this._pane.el.style.setProperty("--number-of-columns", `${ncols}`)
   }
 
-  protected _render_image(item: Item): HTMLCanvasElement {
-    const [_name, colors] = item
-    const {swatch_width, swatch_height} = this.model
-
-    const width = swatch_width
-    const height = swatch_height == "auto" ? swatch_width : swatch_height
-
-    const img = canvas({width, height})
-    const ctx = img.getContext("2d")!
-
-    const n = colors.length
-    const dx = 100.0/n
-
-    for (const [color, i] of enumerate(colors)) {
-      ctx.beginPath()
-      ctx.rect(i*dx, 0, dx, 20)
-      const css_color = color2css(color)
-      ctx.strokeStyle = css_color
-      ctx.fillStyle = css_color
-      ctx.fill()
-      ctx.stroke()
-    }
-
-    return img
-  }
-
   protected _render_item(item: Item): HTMLElement {
-    const [name, colors] = item
-    const {swatch_width, swatch_height} = this.model
-
-    const n = colors.length
-    const stops = linspace(0, 100, n + 1)
-    const color_map: string[] = []
-
-    for (const [color, i] of enumerate(colors)) {
-      const [from, to] = [stops[i], stops[i + 1]]
-      color_map.push(`${color2css(color)} ${from}% ${to}%`)
-    }
-
-    const img = div()
-    img.style.background = `linear-gradient(to right, ${color_map.join(", ")})`
-    img.style.width = px(swatch_width)
-    if (swatch_height == "auto") {
-      img.style.alignSelf = "stretch"
-    } else {
-      img.style.height = px(swatch_height)
-    }
-
-    const entry = div({class: item_css.entry}, img, name)
-    return entry
+    const [name] = item
+    const i = this.model.items.indexOf(item)
+    assert(i != -1)
+    const swatch = div({class: item_css.swatch, id: `item_${i}`})
+    return div({class: item_css.entry}, swatch, div(name))
   }
 
   protected _render_value(): HTMLElement | null {
@@ -134,8 +93,39 @@ export class PaletteSelectView extends InputWidgetView {
   override render(): void {
     super.render()
 
-    const item_els: HTMLElement[] = []
+    const {swatch_width, swatch_height} = this.model
+    this._style.replace(`
+      .${item_css.swatch} {
+        width: ${swatch_width}px;
+        height: ${swatch_height == "auto" ? "auto" : px(swatch_height)};
+      }
+    `)
+
+    for (const [item, i] of enumerate(this.model.items)) {
+      const [, colors] = item
+
+      const n = colors.length
+      const stops = linspace(0, 100, n + 1)
+      const color_map: string[] = []
+
+      for (const [color, i] of enumerate(colors)) {
+        const [from, to] = [stops[i], stops[i + 1]]
+        color_map.push(`${color2css(color)} ${from}% ${to}%`)
+      }
 
+      const gradient = color_map.join(", ")
+      this._style.append(`
+        #item_${i} {
+          background: linear-gradient(to right, ${gradient});
+        }
+      `)
+    }
+
+    // The widget and its menu are independent components, so they need
+    // to have their own stylesheets.
+    this._style_menu.replace(this._style.css)
+
+    const item_els: HTMLElement[] = []
     for (const [item, i] of enumerate(this.model.items)) {
       const entry_el = this._render_item(item)
       const item_el = div({class: item_css.item, tabIndex: 0}, entry_el)
@@ -183,7 +173,7 @@ export class PaletteSelectView extends InputWidgetView {
     this._pane = new DropPane(item_els, {
       target: this.group_el,
       prevent_hide: this.input_el,
-      extra_stylesheets: [item_css.default, pane_css.default],
+      extra_stylesheets: [item_css.default, pane_css.default, this._style_menu],
     })
 
     this._update_ncols()
@@ -253,9 +243,9 @@ export namespace PaletteSelect {
   export type Props = InputWidget.Props & {
     value: p.Property<string>
     items: p.Property<Item[]>
+    ncols: p.Property<number>
     swatch_width: p.Property<number>
     swatch_height: p.Property<number | "auto">
-    ncols: p.Property<number>
   }
 }
 
@@ -272,14 +262,12 @@ export class PaletteSelect extends InputWidget {
   static {
     this.prototype.default_view = PaletteSelectView
 
-    this.define<PaletteSelect.Props>(({Int, Str, List, NonNegative, Positive, Or, Auto}) => {
-      return {
-        value: [ Str ],
-        items: [ List(Item) ],
-        swatch_width: [ NonNegative(Int), 100 ],
-        swatch_height: [ Or(Auto, NonNegative(Int)), "auto" ],
-        ncols: [ Positive(Int), 1 ],
-      }
-    })
+    this.define<PaletteSelect.Props>(({Int, Str, List, NonNegative, Positive, Or, Auto}) => ({
+      value: [ Str ],
+      items: [ List(Item) ],
+      ncols: [ Positive(Int), 1 ],
+      swatch_width: [ NonNegative(Int), 100 ],
+      swatch_height: [ Or(Auto, NonNegative(Int)), "auto" ],
+    }))
   }
 }
diff --git a/...s/test/baselines/linux/Widgets__should_support_PaletteSelect__with_swatch_height=50px.blf b/...s/test/baselines/linux/Widgets__should_support_PaletteSelect__with_swatch_height=50px.blf
@@ -0,0 +1 @@
+PaletteSelect bbox=[0, 0, 200, 56]
diff --git a/...elines/linux/Widgets__should_support_PaletteSelect__with_swatch_height=50px.png b/...elines/linux/Widgets__should_support_PaletteSelect__with_swatch_height=50px.png
diff --git a/...js/test/baselines/linux/Widgets__should_support_PaletteSelect__with_swatch_width=20px.blf b/...js/test/baselines/linux/Widgets__should_support_PaletteSelect__with_swatch_width=20px.blf
@@ -0,0 +1 @@
+PaletteSelect bbox=[0, 0, 120, 31]
diff --git a/...selines/linux/Widgets__should_support_PaletteSelect__with_swatch_width=20px.png b/...selines/linux/Widgets__should_support_PaletteSelect__with_swatch_width=20px.png
diff --git a/bokehjs/test/integration/widgets.ts b/bokehjs/test/integration/widgets.ts
@@ -139,6 +139,22 @@ describe("Widgets", () => {
       await view.ready
     })
 
+    it("with swatch_width=20px", async () => {
+      const obj = new PaletteSelect({value: "Magma", items, ncols: 3, swatch_width: 20})
+      const {view} = await display(obj, [500, 200])
+
+      await tap(view.input_el)
+      await view.ready
+    })
+
+    it("with swatch_height=50px", async () => {
+      const obj = new PaletteSelect({value: "Magma", items, ncols: 3, swatch_height: 50})
+      const {view} = await display(obj, [500, 400])
+
+      await tap(view.input_el)
+      await view.ready
+    })
+
     it("with disabled=true", async () => {
       const obj = new PaletteSelect({value: "Accent", items, disabled: true})
       const {view} = await display(obj, [250, 50])

diff --git a/bokehjs/test/unit/core/util/color.ts b/bokehjs/test/unit/core/util/color.ts
@@ -1,6 +1,6 @@
 import {expect} from "assertions"
 
-import {color2rgba, css4_parse, brightness, luminance} from "@bokehjs/core/util/color"
+import {color2rgba, color2css, css4_parse, brightness, luminance} from "@bokehjs/core/util/color"
 
 describe("core/util/color module", () => {
   const halfgray = color2rgba("rgb(128, 128, 128)")
@@ -131,6 +131,18 @@ describe("core/util/color module", () => {
     })
   })
 
+  describe("implements color2css() function", () => {
+
+    it("which should support CSS colors and maintain their original form", () => {
+      expect(color2css("#00A1FF")).to.be.equal("#00A1FF")
+      expect(color2css("green")).to.be.equal("green")
+      expect(color2css("green", 0.2)).to.be.equal("rgb(0 128 0 / 0.2)")
+      expect(color2css(null)).to.be.equal("rgb(0 0 0 / 0)")
+      expect(color2css([255, 128, 0])).to.be.equal("rgb(255 128 0)")
+      expect(color2css([255, 128, 0, 0.2])).to.be.equal("rgb(255 128 0 / 0.2)")
+    })
+  })
+
   describe("should support css4_parse() function", () => {
     it("that supports 'transparent' keyword", () => {
       expect(css4_parse("")).to.be.null

diff --git a/examples/interaction/widgets/color_map.py b/examples/interaction/widgets/color_map.py
@@ -53,6 +53,7 @@
 color_map0 = PaletteSelect(title="Choose palette:", value="Turbo", items=items0)
 color_map1 = PaletteSelect(title="Choose palette (grid):", value="PuBu", items=items1, ncols=3)
 color_map2 = PaletteSelect(title="Choose palette (disabled):", value="PuBu", items=items1, disabled=True)
+color_map3 = PaletteSelect(title="Choose palette (large swatch):", value="PuBu", items=items1, ncols=3, swatch_height=50)
 
-layout = column(color_map0, color_map1, color_map2)
+layout = column(color_map0, color_map1, color_map2, color_map3)
 show(layout)
diff --git a/tests/unit/bokeh/io/test_export.py b/tests/unit/bokeh/io/test_export.py
@@ -22,6 +22,9 @@
 import sys
 from typing import TYPE_CHECKING
 
+# External imports
+import PIL.Image
+
 ## External imports
 if TYPE_CHECKING:
     from selenium.webdriver.remote.webdriver import WebDriver
@@ -68,6 +71,13 @@ def webdriver_with_scale_factor(request: pytest.FixtureRequest):
     finally:
         webdriver_control.terminate(driver)
 
+@pytest.fixture(scope="module", autouse=True)
+def disable_max_image_pixels():
+    max_image_pixels = PIL.Image.MAX_IMAGE_PIXELS
+    PIL.Image.MAX_IMAGE_PIXELS = None
+    yield
+    PIL.Image.MAX_IMAGE_PIXELS = max_image_pixels
+
 #-----------------------------------------------------------------------------
 # General API
 #-----------------------------------------------------------------------------
@@ -232,9 +242,9 @@ def plot(color: str):
         '<svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="40" height="20">'
             '<defs/>'
             '<path fill="rgb(0,0,0)" stroke="none" paint-order="stroke" d="M 0 0 L 40 0 L 40 20 L 0 20 L 0 0 Z" fill-opacity="0"/>'
-            '<path fill="rgb(255,0,0)" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z" fill-opacity="1"/>'
+            '<path fill="red" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z"/>'
             '<g transform="matrix(1, 0, 0, 1, 20, 0)">'
-                '<path fill="rgb(0,0,255)" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z" fill-opacity="1"/>'
+                '<path fill="blue" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z"/>'
             '</g>'
         '</svg>',
     ]
@@ -266,7 +276,7 @@ def p(color: str):
             return plot
 
         [svg] = bie.get_svg(row([p("red"), p("blue")]), driver=webdriver)
-        assert len(re.findall(r'fill="rgb\(47,63,79\)"', svg)) == 2
+        assert len(re.findall(r'fill="#2f3f4f"', svg)) == 2
     finally:
         state.reset()
 
@@ -298,11 +308,11 @@ def plot(color: str):
     svgs2 = [
         '<svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="20" height="20">'
             '<defs/>'
-            '<path fill="rgb(255,0,0)" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z" fill-opacity="1"/>'
+            '<path fill="red" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z"/>'
         '</svg>',
         '<svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="20" height="20">'
             '<defs/>'
-            '<path fill="rgb(0,0,255)" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z" fill-opacity="1"/>'
+            '<path fill="blue" stroke="none" paint-order="stroke" d="M 5.5 5.5 L 15.5 5.5 L 15.5 15.5 L 5.5 15.5 L 5.5 5.5 Z"/>'
         '</svg>',
     ]