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

Ensure settings are scrolled to using tab navigation #12300

Merged
merged 9 commits into from
Apr 21, 2021
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 20 additions & 43 deletions source/gui/nvdaControls.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
from . import guiHelper
import winUser
import winsound
import math

from collections.abc import Callable


class AutoWidthColumnListCtrl(wx.ListCtrl, listmix.ListCtrlAutoWidthMixin):
"""
A list control that allows you to specify a column to resize to take up the remaining width of a wx.ListCtrl.
Expand Down Expand Up @@ -368,49 +368,26 @@ def GetChildRectRelativeToSelf(self, child: wx.Window) -> wx.Rect:
When calculating ScrollChildIntoView, the position relative to its parent is not relevant unless the
parent is the ScrolledPanel itself. Instead, calculate the position relative to scrolledPanel
"""
cr = child.GetScreenRect()
spr = self.GetScreenPosition()
return wx.Rect(cr.x - spr.x, cr.y - spr.y, cr.width, cr.height)
childRectRelativeToScreen = child.GetScreenRect()
scrolledPanelScreenPosition = self.GetScreenPosition()
return wx.Rect(
childRectRelativeToScreen.x - scrolledPanelScreenPosition.x,
childRectRelativeToScreen.y - scrolledPanelScreenPosition.y,
childRectRelativeToScreen.width,
childRectRelativeToScreen.height
)

def ScrollChildIntoView(self, child: wx.Window) -> None:
"""
Uses the same logic as super().ScrollChildIntoView(self, child)
except cr = self.GetChildRectRelativeToSelf(child) instead of cr = GetRect()
Overrides child.GetRect with `GetChildRectRelativeToSelf` before calling
`super().ScrollChildIntoView`. `super().ScrollChildIntoView` incorrectly uses child.GetRect to
navigate scrolling, which is relative to the parent, where it should instead be relative to this
ScrolledPanel.
"""
sppu_x, sppu_y = self.GetScrollPixelsPerUnit()
vs_x, vs_y = self.GetViewStart()
cr = self.GetChildRectRelativeToSelf(child)
clntsz = self.GetClientSize()
new_vs_x, new_vs_y = -1, -1

# is it before the left edge?
if cr.x < 0 and sppu_x > 0:
new_vs_x = vs_x + (cr.x / sppu_x)

# is it above the top?
if cr.y < 0 and sppu_y > 0:
new_vs_y = vs_y + (cr.y / sppu_y)

# For the right and bottom edges, scroll enough to show the
# whole control if possible, but if not just scroll such that
# the top/left edges are still visible

# is it past the right edge ?
if cr.right > clntsz.width and sppu_x > 0:
diff = math.ceil(1.0 * (cr.right - clntsz.width + 1) / sppu_x)
if cr.x - diff * sppu_x > 0:
new_vs_x = vs_x + diff
else:
new_vs_x = vs_x + (cr.x / sppu_x)

# is it below the bottom ?
if cr.bottom > clntsz.height and sppu_y > 0:
diff = math.ceil(1.0 * (cr.bottom - clntsz.height + 1) / sppu_y)
if cr.y - diff * sppu_y > 0:
new_vs_y = vs_y + diff
else:
new_vs_y = vs_y + (cr.y / sppu_y)

# if we need to adjust
if new_vs_x != -1 or new_vs_y != -1:
self.Scroll(new_vs_x, new_vs_y)
oldChildGetRectFunction = child.GetRect
child.GetRect = lambda: self.GetChildRectRelativeToSelf(child)
try:
super().ScrollChildIntoView(child)
finally:
# ensure child.GetRect is reset properly even if super().ScrollChildIntoView throws an exception
child.GetRect = oldChildGetRectFunction
Comment on lines +388 to +393
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I avoided suggesting this because it's a little dangerous. If any of the other method calls within super().ScrollChildIntoView rely on the absolute value for rect for the child, this will be broken in a subtle way.
Worse, by the time we update wxPython, we may have forgotten about this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

child is only used once in super().ScrollChildIntoView, and that is to calculate the incorrect child.GetRect https://github.com/wxWidgets/Phoenix/blob/wxPython-4.1.1/wx/lib/scrolledpanel.py#L190. Do we only update wxPython with compatibility breaking releases? That way I can mark this as deprecated for 2022.1 so it won't get lost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense that we only update on compat breaking releases. Let's mark it as deprecated.