Improve respect for maintain_focus = False when zooming (bokeh#14000)

* Improve respect for maintain_focus zoom option * Add integration regression tests
abdulrahman305 · Jul 29, 2024 · b902fb5 · b902fb5
1 parent 18ee70b
commit b902fb5
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 14 deletions.
diff --git a/bokehjs/src/lib/models/plots/gmap_plot_canvas.ts b/bokehjs/src/lib/models/plots/gmap_plot_canvas.ts
@@ -101,7 +101,7 @@ export class GMapPlotView extends PlotView {
     super.remove()
   }
 
-  override update_range(range_info: GMapRangeInfo | null, options?: RangeOptions): void {
+  override update_range(range_info: GMapRangeInfo | null, options?: Partial<RangeOptions>): void {
     // RESET -------------------------
     if (range_info == null) {
       this.map.setCenter({lat: this.initial_lat, lng: this.initial_lng})

diff --git a/bokehjs/src/lib/models/plots/plot_canvas.ts b/bokehjs/src/lib/models/plots/plot_canvas.ts
@@ -642,7 +642,7 @@ export class PlotView extends LayoutDOMView implements Paintable {
     return views
   }
 
-  update_range(range_info: RangeInfo, options?: RangeOptions): void {
+  update_range(range_info: RangeInfo, options?: Partial<RangeOptions>): void {
     this.pause()
     this._range_manager.update(range_info, options)
     this.unpause()

diff --git a/bokehjs/src/lib/models/plots/range_manager.ts b/bokehjs/src/lib/models/plots/range_manager.ts
@@ -15,9 +15,9 @@ export type RangeInfo = {
 }
 
 export type RangeOptions = {
-  panning?: boolean
-  scrolling?: boolean
-  maintain_focus?: boolean
+  panning: boolean
+  scrolling: boolean
+  maintain_focus: boolean
 }
 
 export class RangeManager {
@@ -29,18 +29,23 @@ export class RangeManager {
 
   invalidate_dataranges: boolean = true
 
-  update(range_info: RangeInfo, options: RangeOptions = {}): void {
+  update(range_info: RangeInfo, options: Partial<RangeOptions> = {}): void {
+    const panning = options.panning ?? false
+    const scrolling = options.scrolling ?? false
+    const maintain_focus = options.maintain_focus ?? false
+
     const range_state: RangeState = new Map()
     for (const [range, interval] of range_info.xrs) {
       range_state.set(range, interval)
     }
     for (const [range, interval] of range_info.yrs) {
       range_state.set(range, interval)
     }
-    if (options.scrolling ?? false) {
+
+    if (scrolling && maintain_focus) {
       this._update_ranges_together(range_state)   // apply interval bounds while keeping aspect
     }
-    this._update_ranges_individually(range_state, options)
+    this._update_ranges_individually(range_state, {panning, scrolling, maintain_focus})
   }
 
   ranges(): {x_ranges: Range[], y_ranges: Range[]} {
@@ -216,16 +221,14 @@ export class RangeManager {
     }
   }
 
-  protected _update_ranges_individually(range_state: RangeState, options: RangeOptions = {}): void {
-    const panning = options.panning ?? false
-    const scrolling = options.scrolling ?? false
-    const maintain_focus = options.maintain_focus ?? false
+  protected _update_ranges_individually(range_state: RangeState, options: RangeOptions): void {
+    const {panning, scrolling, maintain_focus} = options
 
     let hit_bound = false
     for (const [rng, range_info] of range_state) {
       // Limit range interval first. Note that for scroll events,
       // the interval has already been limited for all ranges simultaneously
-      if (!scrolling) {
+      if (!scrolling || maintain_focus) {
         const weight = this._get_weight_to_constrain_interval(rng, range_info)
         if (weight < 1) {
           range_info.start = weight*range_info.start + (1 - weight)*rng.start

diff --git a/...inux/Bug__in_issue_#13827__doesn't_allow_to_respect_maintain_focus=false_when_zooming.blf b/...inux/Bug__in_issue_#13827__doesn't_allow_to_respect_maintain_focus=false_when_zooming.blf
@@ -0,0 +1,7 @@
+Figure bbox=[0, 0, 200, 200]
+  Canvas bbox=[0, 0, 200, 200]
+  CartesianFrame bbox=[20, 5, 175, 173]
+  GlyphRenderer bbox=[20, 5, 175, 173]
+    Line bbox=[20, 5, 175, 173]
+  LinearAxis bbox=[20, 178, 175, 22]
+  LinearAxis bbox=[0, 5, 20, 173]
diff --git a/...in_issue_#13827__doesn't_allow_to_respect_maintain_focus=false_when_zooming.png b/...in_issue_#13827__doesn't_allow_to_respect_maintain_focus=false_when_zooming.png
diff --git a/bokehjs/test/integration/regressions.ts b/bokehjs/test/integration/regressions.ts
@@ -17,7 +17,8 @@ import {
   LinearColorMapper,
   Plot,
   TeX,
-  Toolbar, ToolProxy, PanTool, PolySelectTool, LassoSelectTool, HoverTool, ZoomInTool, ZoomOutTool, RangeTool, WheelPanTool,
+  Toolbar, ToolProxy,
+  PanTool, PolySelectTool, LassoSelectTool, HoverTool, ZoomInTool, ZoomOutTool, RangeTool, WheelPanTool, WheelZoomTool,
   TileRenderer, WMTSTileSource,
   ImageURLTexture,
   Row, Column, Spacer,
@@ -4056,4 +4057,31 @@ describe("Bug", () => {
       await view.ready
     })
   })
+
+  describe("in issue #13827", () => {
+    it("doesn't allow to respect maintain_focus=false when zooming", async () => {
+      const p = fig([200, 200], {
+        x_range: new Range1d({bounds: [1, 5], start: 1, end: 2}),
+        y_range: new Range1d({bounds: [2, 7], start: 4, end: 6.5}),
+        tools: "reset,pan",
+      })
+
+      p.line({
+        x: [1, 2, 3, 4, 5],
+        y: [6, 7, 2, 4, 5],
+      })
+
+      const wheel_zoom = new WheelZoomTool({maintain_focus: false})
+      p.add_tools(wheel_zoom)
+      p.toolbar.active_scroll = wheel_zoom
+
+      const {view} = await display(p)
+      const ac = actions(view, {units: "screen"})
+
+      for (const _ of range(0, 10)) {
+        await ac.scroll_down(xy(100, 100))
+        await view.ready
+      }
+    })
+  })
 })