Text box can be used to set DPI (#960)

* Add dpi_entry GtkEntry objects to UI file and Python.
Find and apply closest resolution in self.resolutions.
[Needed?] Store value on textbox focus in.
[Needed?] Do not apply DPI value if the same as previous value.
DPI entry and resolution sliders affect each others' values.

* Remove pointless recursive logic.
Remove focus methods.

* Calculate DPI text box width based on max supported resolution.

* Renamed methods/signals to be consistent with current convention.

* Add a 'focus-in-event' signal to the DPI entry field.
Pass reference to ResolutionsPage when initialising ResolutionRow.
Use reference to invoke `ResolutionsPage._on_row_activated()`.
Do not toggle Revealer on DPI entry text box focus-in-event if Revealer is already expanded.
Grammar: dpi -> DPI

* Add type hints.
Prefix unused vars with underscores.
Actioned suggested import order from linter.

* Actually apply the closest res value (oops).
Apply the res value on focus-out-event.

* Toggle allow editing of DPI entry if 'disable' button clicked.

* Use set_sensitive() on DPI entry.

* rm pointless var.

* Create custom class for DPI text entry to circumvent assertion warning.
Grab DPIEntry focus when revealing ResolutionRow.
Revert import order.
Removed unused 'previous DPI value' var.
Remove 'private' convention underscore from ResolutionsPage._on_row_activate().
Remove callback method for DPI entry on ResolutionRow.
UI file uses DPIEntry.
Remove insert-text signal for DPIEntry from UI file.

* Line lengths

* Removed unused import.
Underscored unused parameter (PEP convention).

* Reverted underscore convention as to not break existing stuff.

* Use underscore convention, but also use it when invoking the method.

* Renamed handler name for ResolutionsPage to match method.

* Clarified docstring.

---------

Co-authored-by: Dan <[email protected]>

data/ui/ResolutionRow.ui

@@ -16,12 +16,21 @@
             <property name="can-focus">False</property>
             <property name="border-width">12</property>
             <property name="spacing">6</property>
+            <child>
+              <object class="DPIEntry" id="dpi_entry">
+                <property name="visible">True</property>
+                <property name="halign">start</property>
+                <signal name="activate" handler="_on_dpi_entry_activate" swapped="no"/>
+                <signal name="focus-in-event" handler="_on_dpi_entry_focus_in" swapped="no"/>
+                <signal name="focus-out-event" handler="_on_dpi_entry_focus_out" swapped="no"/>
+              </object>
+            </child>
             <child>
               <object class="GtkLabel" id="dpi_label">
                 <property name="visible">True</property>
                 <property name="can-focus">False</property>
                 <property name="halign">start</property>
-                <property name="label">0 DPI</property>
+                <property name="label">DPI</property>
               </object>
               <packing>
                 <property name="expand">True</property>
@@ -66,7 +75,7 @@
                   <object class="GtkScale" id="scale">
                     <property name="visible">True</property>
                     <property name="can-focus">True</property>
-                    <property name="tooltip-text" translatable="yes">Set this resolution's dpi</property>
+                    <property name="tooltip-text" translatable="yes">Set this resolution's DPI</property>
                     <property name="round-digits">0</property>
                     <property name="digits">0</property>
                     <property name="draw-value">False</property>

data/ui/ResolutionsPage.ui

@@ -38,7 +38,7 @@
                         <property name="visible">True</property>
                         <property name="can-focus">False</property>
                         <property name="selection-mode">none</property>
-                        <signal name="row-activated" handler="_on_row_activated" swapped="no"/>
+                        <signal name="row-activated" handler="on_row_activated" swapped="no"/>
                         <child>
                           <object class="GtkListBoxRow" id="add_resolution_row">
                             <property name="can-focus">True</property>

piper/resolutionrow.py

@@ -11,6 +11,34 @@
 from gi.repository import GObject, Gdk, Gtk  # noqa
 
 
+class _DPIEntry(Gtk.Entry, Gtk.Editable):
+    """
+    A subclass of Gtk.Entry with an overridden method for input validation.
+
+    This class addresses a specific bug in the handling of parameters in pygobject when
+    using signals. For Gtk.Entry, the 'position' parameter is incorrect when handling
+    the 'insert-text' signal, causing an assertion warning to be logged. By using this
+    subclass, we can override the 'insert-text' logic to correctly handle the position
+    parameter and avoid the warning.
+
+    Inheriting from Gtk.Editable in addition to Gtk.Entry ensures that the overridden
+    method applies only to instances of this class, rather than affecting all
+    Gtk.Entry objects in the application.
+    """
+
+    __gtype_name__ = "DPIEntry"
+
+    def do_insert_text(self, new_text: str, new_text_length: int, position: int) -> int:
+        """Overrides the default Gtk.Editable insert-text handler to validate numerical
+        input."""
+        if not new_text.isdigit():
+            self.stop_emission("insert-text")
+            return position
+
+        self.get_buffer().insert_text(position, new_text, new_text_length)
+        return position + new_text_length
+
+
 @Gtk.Template(resource_path="/org/freedesktop/Piper/ui/ResolutionRow.ui")
 class ResolutionRow(Gtk.ListBoxRow):
     """A Gtk.ListBoxRow subclass containing the widgets to configure a
@@ -22,15 +50,19 @@ class ResolutionRow(Gtk.ListBoxRow):
     active_label: Gtk.Label = Gtk.Template.Child()  # type: ignore
     disable_button: Gtk.Button = Gtk.Template.Child()  # type: ignore
     dpi_label: Gtk.Label = Gtk.Template.Child()  # type: ignore
+    dpi_entry: _DPIEntry = Gtk.Template.Child()  # type: ignore
     revealer: Gtk.Revealer = Gtk.Template.Child()  # type: ignore
     scale: Gtk.Scale = Gtk.Template.Child()  # type: ignore
 
     CAP_SEPARATE_XY_RESOLUTION = False
     CAP_DISABLE = False
 
-    def __init__(self, resolution: RatbagdResolution, *args, **kwargs) -> None:
+    def __init__(
+        self, resolution: RatbagdResolution, resolutions_page, *args, **kwargs
+    ) -> None:
         Gtk.ListBoxRow.__init__(self, *args, **kwargs)
 
+        self.resolutions_page = resolutions_page
         self._resolution = resolution
         self.resolutions = resolution.resolutions
         self._scale_handler = self.scale.connect(
@@ -60,13 +92,19 @@ def __init__(self, resolution: RatbagdResolution, *args, **kwargs) -> None:
         res = resolution.resolution[0]
         minres = resolution.resolutions[0]
         maxres = resolution.resolutions[-1]
+        res_num_chars = len(str(maxres))
+
+        # Set the max width and length for the DPI text boxes, based on the max res value.
+        self.dpi_entry.set_width_chars(res_num_chars)
+        self.dpi_entry.set_max_length(res_num_chars)
+
         with self.scale.handler_block(self._scale_handler):
             self.scale.props.adjustment.configure(res, minres, maxres, 50, 50, 0)
             self.scale.set_value(res)
         if resolution.is_disabled:
             with self.disable_button.handler_block(self._disabled_button_handler):
                 self.disable_button.set_active(True)
-        self._on_status_changed(resolution, pspec=None)
+        self._on_status_changed(resolution, _pspec=None)
 
     @Gtk.Template.Callback("_on_change_value")
     def _on_change_value(
@@ -82,6 +120,7 @@ def _on_change_value(
         value = lo if value - lo < hi - value else hi
 
         scale.set_value(value)
+        self.dpi_entry.set_text(str(value))
 
         # libratbag provides a fake-exponential range with the deltas
         # increasing as the resolution goes up. Make sure we set our
@@ -99,15 +138,15 @@ def _on_disable_button_toggled(self, togglebutton: Gtk.Button) -> None:
         self._resolution.set_disabled(togglebutton.get_active())
 
         # Update UI
-        self._on_status_changed(self._resolution, pspec=None)
+        self._on_status_changed(self._resolution, _pspec=None)
 
     @Gtk.Template.Callback("_on_active_button_clicked")
-    def _on_active_button_clicked(self, togglebutton: Gtk.Button) -> None:
+    def _on_active_button_clicked(self, _togglebutton: Gtk.Button) -> None:
         # The set active button has been clicked, update RatbagdResolution.
         self._resolution.set_active()
 
     @Gtk.Template.Callback("_on_scroll_event")
-    def _on_scroll_event(self, widget: Gtk.Widget, event: Gdk.EventScroll) -> bool:
+    def _on_scroll_event(self, widget: Gtk.Widget, _event: Gdk.EventScroll) -> bool:
         # Prevent a scroll in the list to get caught by the scale.
         GObject.signal_stop_emission_by_name(widget, "scroll-event")
         return False
@@ -118,7 +157,7 @@ def _on_scale_value_changed(self, scale: Gtk.Scale) -> None:
         self._on_dpi_values_changed(res=res)
 
     def _on_status_changed(
-        self, resolution: RatbagdResolution, pspec: Optional[GObject.ParamSpec]
+        self, resolution: RatbagdResolution, _pspec: Optional[GObject.ParamSpec]
     ) -> None:
         # The resolution's status changed, update UI.
         self._on_dpi_values_changed()
@@ -136,33 +175,63 @@ def _on_status_changed(
                         self.disable_button.set_active(True)
                         self.active_button.set_sensitive(False)
                         self.dpi_label.set_sensitive(False)
+                        self.dpi_entry.set_sensitive(False)
                         self.scale.set_sensitive(False)
                     else:
                         self.disable_button.set_active(False)
                         self.dpi_label.set_sensitive(True)
+                        self.dpi_entry.set_sensitive(True)
                         self.scale.set_sensitive(True)
 
     def toggle_revealer(self) -> None:
         # Toggles the revealer to show or hide the configuration widgets.
         reveal = not self.revealer.get_reveal_child()
         self.revealer.set_reveal_child(reveal)
+        if reveal:
+            self.dpi_entry.grab_focus()
 
     def _on_dpi_values_changed(self, res: Optional[int] = None) -> None:
         # Freeze the notify::resolution signal from firing and
-        # update dpi label and resolution values.
+        # update DPI text box and resolution values.
         if res is None:
             res = self._resolution.resolution[0]
         new_res = (res, res) if self.CAP_SEPARATE_XY_RESOLUTION else (res,)
-        self.dpi_label.set_text(f"{res} DPI")
+        self.dpi_entry.set_text(str(res))
 
         # Only update new resolution if changed
         if new_res != self._resolution.resolution:
             self._resolution.resolution = new_res
 
+    @Gtk.Template.Callback("_on_dpi_entry_activate")
+    def _on_dpi_entry_activate(self, entry: Gtk.Entry) -> None:
+        # The DPI entry has been changed, update the scale and RatbagdResolution's resolution.
+        try:
+            res = int(entry.get_text())
+            # Get the resolution closest to what has been entered
+            closest_res = min(self.resolutions, key=lambda x: abs(x - res))
+            entry.set_text(str(closest_res))
+            self._on_dpi_values_changed(res=closest_res)
+
+            with self.scale.handler_block(self._scale_handler):
+                self.scale.set_value(res)
+        except ValueError:
+            # If the input is not a valid integer, reset to the current value.
+            entry.set_text(str(self._resolution.resolution[0]))
+
+    @Gtk.Template.Callback("_on_dpi_entry_focus_in")
+    def _on_dpi_entry_focus_in(self, _entry: Gtk.Entry, _event_focus: Gdk.EventFocus):
+        if not self.revealer.get_reveal_child():
+            self.resolutions_page.on_row_activated(None, self)
+
+    @Gtk.Template.Callback("_on_dpi_entry_focus_out")
+    def _on_dpi_entry_focus_out(self, entry: Gtk.Entry, _event_focus: Gdk.EventFocus):
+        # Apply the DPI value on focus out
+        self._on_dpi_entry_activate(entry)
+
     def _on_profile_resolution_changed(
-        self, resolution: RatbagdResolution, pspec: GObject.ParamSpec
+        self, resolution: RatbagdResolution, _pspec: GObject.ParamSpec
     ) -> None:
         with self.scale.handler_block(self._scale_handler):
             res = resolution.resolution[0]
             self.scale.set_value(res)
-            self.dpi_label.set_text(f"{res} DPI")
+            self.dpi_entry.set_text(str(res))

piper/resolutionspage.py

@@ -63,11 +63,11 @@ def __init__(
 
         self.listbox.foreach(Gtk.Widget.destroy)
         for resolution in profile.resolutions:
-            row = ResolutionRow(resolution)
+            row = ResolutionRow(resolution, self)
             self.listbox.insert(row, resolution.index)
 
-    @Gtk.Template.Callback("_on_row_activated")
-    def _on_row_activated(self, listbox: Gtk.ListBox, row: ResolutionRow) -> None:
+    @Gtk.Template.Callback("on_row_activated")
+    def on_row_activated(self, _listbox: Gtk.ListBox, row: ResolutionRow) -> None:
         if row is self._last_activated_row:
             self._last_activated_row = None
             row.toggle_revealer()