Repository: spyder-ide/spyder
Pull request: PR: Stop automatic scrolling when a plot is selected (Plots)
<!--- Make sure to read the Contributing Guidelines:                   --->
<!--- https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md --->
<!--- and follow PEP 8, PEP 257 and Spyder's code style:               --->
<!--- https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style      --->

## Description of Changes
Only scroll down in the plots plane if the latest plot is selected or if a new execution started



* [ ] Wrote at least one-line docstrings (for any new functions)
* [ ] Added unit test(s) covering the changes (if testable)
<!--- Remember that an image/animation is worth a thousand words! --->
* [ ] Included a screenshot or animation (if affecting the UI, see [Licecap](https://www.cockos.com/licecap/))


<!--- Explain what you've done and why --->




### Issue(s) Resolved

<!--- List the issue(s) below, in the form "Fixes #1234"; one per line --->

Fixes #20474


### Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the [Developer Certificate of Origin](https://developercertificate.org)
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

<!--- TYPE YOUR USER/NAME AFTER THE FOLLOWING: --->
I certify the above statement is true and correct:

<!--- Thanks for your help making Spyder better for everyone! --->


Problem statement:
Generating new plots always overrides currently selected plot
Consider the following code:

```
import matplotlib.pyplot as plt, time
for i in range(100):
    plt.figure()
    plt.plot([0, 1, i])
    plt.show()
    time.sleep(0.1)
```

Every time a new plot is generated, it is selected in the plots panel. If you want to look at a plot, you must wait untill all the plots are generated.

It would be great if, when you select a plot, the selected plot is not replaced by a newly plotted plot.

Hints from issue discussion:
Checked locally and that makes sense to me :+1: What do you think @spyder-ide/core-developers ?
Generally makes sense to me in that particular case, but my concern is that if a user had a fair number of plots and had some older plot selected from well before they executed their code, they might expect Spyder to still open their present plot automatically and not see/realize new plots had been added, and either go looking for their plot and not be sure where to find it, or think the previous plot was their new plot—especially if the plots pane was not visible initially when they ran their plotting code.

To more carefully fix the specific problem presented without running into these issues, we could suppress switching to a new plot only if a selection was made after the current code started executing. I.e., if I run my code and then select a particular plot, it is not overridden, but if I just happened to have a plot selected at some point prior, then still show the new plots until I actively select something else.

If that's too difficult to implement or is still not enough, an alternative would be disabling switching to a different plot when the plot pane is visible (i.e. not hidden, and on the currently active tab in a panel) and a plot is selected, which would not be always quite so obvious, but more so than if it were not active or hidden.
What I implemented in https://github.com/spyder-ide/spyder/pull/20475 is:
If the user does nothing, select the last plot
If a plot is selected during execution, do no replace it so that the user can look at it
If a new plot is created after executing (first plot after a new execution) select this one and start scrolling again until the user selects a plot
> To more carefully fix the specific problem presented without running into these issues, we could suppress switching to a new plot only if a selection was made after the current code started executing

I think what @impact27 did fulfills this requirement, right @CAM-Gerlach?

Commit message: Merge remote-tracking branch 'upstream/master' into stick_to_end_plots

Patch to review:
diff --git a/spyder/app/tests/test_mainwindow.py b/spyder/app/tests/test_mainwindow.py
index 38cd066069c..ef4f811c6eb 100644
--- a/spyder/app/tests/test_mainwindow.py
+++ b/spyder/app/tests/test_mainwindow.py
@@ -2049,6 +2049,80 @@ def test_plots_plugin(main_window, qtbot, tmpdir, mocker):
     assert compare_images(ipython_figname, plots_figname, 0.1) is None
 
 
+def test_plots_scroll(main_window, qtbot):
+    """Test plots plugin scrolling"""
+    CONF.set('plots', 'mute_inline_plotting', True)
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    figbrowser = main_window.plots.current_widget()
+
+    # Wait until the window is fully up.
+    qtbot.waitUntil(
+        lambda: shell.spyder_kernel_ready and shell._prompt_html is not None,
+        timeout=SHELL_TIMEOUT)
+
+    # Generate a plot inline.
+    with qtbot.waitSignal(shell.executed, timeout=SHELL_TIMEOUT):
+        shell.execute(("import matplotlib.pyplot as plt\n"
+                       "fig = plt.plot([1, 2, 3, 4], '.')\n"))
+
+    # Make sure the plot is selected
+    sb = figbrowser.thumbnails_sb
+    assert len(sb._thumbnails) == 1
+    assert sb._thumbnails[-1] == sb.current_thumbnail
+
+    # Generate 4 more plots
+    with qtbot.waitSignal(shell.executed, timeout=SHELL_TIMEOUT):
+        shell.execute(
+            "for i in range(4):\n"
+            "    plt.figure()\n"
+            "    plt.plot([1, 2, 3, 4], '.')")
+
+    # we now have 5 plots and the last one is selected
+    assert len(sb._thumbnails) == 5
+    assert sb._thumbnails[-1] == sb.current_thumbnail
+
+    # Generate 20 plots
+    with qtbot.waitSignal(shell.executed, timeout=SHELL_TIMEOUT):
+        shell.execute(
+            "for i in range(20):\n"
+            "    plt.figure()\n"
+            "    plt.plot([1, 2, 3, 4], '.')")
+
+    # Make sure we scrolled down
+    scrollbar = sb.scrollarea.verticalScrollBar()
+    assert len(sb._thumbnails) == 25
+    assert sb._thumbnails[-1] == sb.current_thumbnail
+    assert scrollbar.value() == scrollbar.maximum()
+
+    # Generate 20 plots and select a plot in the middle
+    with qtbot.waitSignal(shell.executed, timeout=SHELL_TIMEOUT):
+        shell.execute(
+            "import time\n"
+            "for i in range(20):\n"
+            "    plt.figure()\n"
+            "    plt.plot([1, 2, 3, 4], '.')\n"
+            "    plt.show()\n"
+            "    time.sleep(.1)")
+        qtbot.waitUntil(lambda: sb._first_thumbnail_shown, timeout=SHELL_TIMEOUT)
+        sb.set_current_index(5)
+        scrollbar.setValue(scrollbar.minimum())
+
+    # make sure we didn't scroll to the end and a new thumbnail was not selected
+    assert len(sb._thumbnails) == 45
+    assert sb._thumbnails[-1] != sb.current_thumbnail
+    assert scrollbar.value() != scrollbar.maximum()
+
+    # One more plot
+    with qtbot.waitSignal(shell.executed, timeout=SHELL_TIMEOUT):
+        shell.execute(("fig = plt.plot([1, 2, 3, 4], '.')\n"))
+
+    # Make sure everything scrolled at the end
+    assert len(sb._thumbnails) == 46
+    assert sb._thumbnails[-1] == sb.current_thumbnail
+    assert scrollbar.value() == scrollbar.maximum()
+    CONF.set('plots', 'mute_inline_plotting', False)
+
+
 @flaky(max_runs=3)
 @pytest.mark.skipif(
     (parse_version(ipy_release.version) >= parse_version('7.23.0') and
diff --git a/spyder/plugins/plots/widgets/figurebrowser.py b/spyder/plugins/plots/widgets/figurebrowser.py
index c619feb193e..c3591e8438e 100644
--- a/spyder/plugins/plots/widgets/figurebrowser.py
+++ b/spyder/plugins/plots/widgets/figurebrowser.py
@@ -243,6 +243,7 @@ def set_shellwidget(self, shellwidget):
         self.shellwidget = shellwidget
         self.shellwidget.set_mute_inline_plotting(self.mute_inline_plotting)
         shellwidget.sig_new_inline_figure.connect(self._handle_new_figure)
+        shellwidget.executing.connect(self._handle_new_execution)
 
     def _handle_new_figure(self, fig, fmt):
         """
@@ -251,6 +252,10 @@ def _handle_new_figure(self, fig, fmt):
         """
         self.thumbnails_sb.add_thumbnail(fig, fmt)
 
+    def _handle_new_execution(self):
+        """Handle a new execution in the console."""
+        self.thumbnails_sb._first_thumbnail_shown = False
+
     # ---- Toolbar Handlers
     def zoom_in(self):
         """Zoom the figure in by a single step in the figure viewer."""
@@ -587,7 +592,8 @@ def __init__(self, figure_viewer, parent=None, background_color=None):
         # is added to the scrollarea, we need to do it instead in a slot
         # connected to the scrollbar's rangeChanged signal.
         # See spyder-ide/spyder#10914 for more details.
-        self._new_thumbnail_added = False
+        self._scroll_to_last_thumbnail = False
+        self._first_thumbnail_shown = False
         self.scrollarea.verticalScrollBar().rangeChanged.connect(
             self._scroll_to_newest_item)
 
@@ -768,6 +774,15 @@ def add_thumbnail(self, fig, fmt):
         """
         Add a new thumbnail to that thumbnail scrollbar.
         """
+        # Always stick at end for the first thumbnail
+        is_first = not self._first_thumbnail_shown
+
+        # Stick at end if we are already at the end
+        stick_at_end = False
+        sb = self.scrollarea.verticalScrollBar()
+        if sb.value() == sb.maximum():
+            stick_at_end = True
+
         thumbnail = FigureThumbnail(
             parent=self, background_color=self.background_color)
         thumbnail.canvas.load_figure(fig, fmt)
@@ -777,16 +792,28 @@ def add_thumbnail(self, fig, fmt):
         thumbnail.sig_context_menu_requested.connect(
             lambda point: self.show_context_menu(point, thumbnail))
         self._thumbnails.append(thumbnail)
-        self._new_thumbnail_added = True
+        self._scroll_to_last_thumbnail = True
+        self._first_thumbnail_shown = True
 
         self.scene.setRowStretch(self.scene.rowCount() - 1, 0)
         self.scene.addWidget(thumbnail, self.scene.rowCount() - 1, 0)
         self.scene.setRowStretch(self.scene.rowCount(), 100)
-        self.set_current_thumbnail(thumbnail)
+
+        # Only select a new thumbnail if the last one was selected
+        select_last = (
+            len(self._thumbnails) < 2
+            or self.current_thumbnail == self._thumbnails[-2]
+            or is_first
+        )
+        if select_last:
+            self.set_current_thumbnail(thumbnail)
 
         thumbnail.show()
         self._setup_thumbnail_size(thumbnail)
 
+        if not is_first and not stick_at_end:
+            self._scroll_to_last_thumbnail = False
+
     def remove_current_thumbnail(self):
         """Remove the currently selected thumbnail."""
         if self.current_thumbnail is not None:
@@ -912,8 +939,8 @@ def _scroll_to_newest_item(self, vsb_min, vsb_max):
         Note that this method is called each time the rangeChanged signal
         is emitted by the scrollbar.
         """
-        if self._new_thumbnail_added:
-            self._new_thumbnail_added = False
+        if self._scroll_to_last_thumbnail:
+            self._scroll_to_last_thumbnail = False
             self.scrollarea.verticalScrollBar().setValue(vsb_max)
 
     # ---- ScrollBar Handlers


Code review comments:
diff_hunk: @@ -2102,7 +2103,7 @@ def test_plots_scroll(main_window, qtbot):
             "    plt.plot([1, 2, 3, 4], '.')\n"
             "    plt.show()\n"
             "    time.sleep(.1)")
-        qtbot.wait(500)
+        qtbot.waitUntil(lambda: sb._first_thumbnail_shown, timeout=SHELL_TIMEOUT)
path: spyder/app/tests/test_mainwindow.py
text: @user1:
```suggestion
        qtbot.waitUntil(lambda: sb._first_thumbnail_shown,
                        timeout=SHELL_TIMEOUT)
```