Address review comments

Author: DataTriny

bindings/python/examples/pygame/README.md

@@ -4,7 +4,7 @@ This directory contains a cross-platform application that demonstrates how to in
 
 ## Prerequisites
 
-- Python 3.7 or higher
+- Python 3.9 or higher
 - A virtual environment: `python -m venv .venv` (activating it will vary based on your platform)
 - `pip install -r requirements.txt` 
 

bindings/python/src/common.rs

@@ -3,6 +3,7 @@
 // the LICENSE-APACHE file) or the MIT license (found in
 // the LICENSE-MIT file), at your option.
 
+use crate::{Point, Rect};
 use pyo3::{prelude::*, types::PyList};
 
 #[pyclass(module = "accesskit")]
@@ -257,7 +258,7 @@ macro_rules! simple_getter {
     };
 }
 
-macro_rules! convertion_getter {
+macro_rules! converting_getter {
     ($struct_name:ident, $getter:ident, $type:ty) => {
         #[pymethods]
         impl $struct_name {
@@ -292,7 +293,7 @@ macro_rules! simple_setter {
     };
 }
 
-macro_rules! convertion_setter {
+macro_rules! converting_setter {
     ($setter:ident, $setter_param:ty) => {
         #[pymethods]
         impl NodeBuilder {
@@ -379,7 +380,7 @@ macro_rules! node_id_vec_property_methods {
 macro_rules! node_id_property_methods {
     ($(($getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
-            ($getter, option_getter, Option<NodeId>, $setter, convertion_setter, NodeId, $clearer)
+            ($getter, option_getter, Option<NodeId>, $setter, converting_setter, NodeId, $clearer)
         })*
     }
 }
@@ -411,15 +412,15 @@ macro_rules! color_property_methods {
 macro_rules! text_decoration_property_methods {
     ($(($getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
-            ($getter, option_getter, Option<accesskit::TextDecoration>, $setter, convertion_setter, accesskit::TextDecoration, $clearer)
+            ($getter, option_getter, Option<accesskit::TextDecoration>, $setter, converting_setter, accesskit::TextDecoration, $clearer)
         })*
     }
 }
 
 macro_rules! length_slice_property_methods {
     ($(($getter:ident, $setter:ident, $clearer:ident)),+) => {
         $(property_methods! {
-            ($getter, convertion_getter, Vec<u8>, $setter, simple_setter, Vec<u8>, $clearer)
+            ($getter, converting_getter, Vec<u8>, $setter, simple_setter, Vec<u8>, $clearer)
         })*
     }
 }
@@ -588,7 +589,7 @@ unique_enum_property_methods! {
 
 property_methods! {
     (transform, option_getter, Option<crate::Affine>, set_transform, simple_setter, crate::Affine, clear_transform),
-    (bounds, option_getter, Option<crate::Rect>, set_bounds, convertion_setter, crate::Rect, clear_bounds),
+    (bounds, option_getter, Option<crate::Rect>, set_bounds, converting_setter, crate::Rect, clear_bounds),
     (text_selection, option_getter, Option<TextSelection>, set_text_selection, simple_setter, TextSelection, clear_text_selection)
 }
 
@@ -602,7 +603,7 @@ pub struct Tree {
     pub root: NodeId,
     pub app_name: Option<String>,
     pub toolkit_name: Option<String>,
-    toolkit_version: Option<String>,
+    pub toolkit_version: Option<String>,
 }
 
 #[pymethods]
@@ -674,68 +675,55 @@ impl From<TreeUpdate> for accesskit::TreeUpdate {
     }
 }
 
-#[pyclass(module = "accesskit")]
-pub struct ActionData(accesskit::ActionData);
-
-#[pymethods]
-impl ActionData {
-    #[staticmethod]
-    pub fn custom_action(action: i32) -> Self {
-        accesskit::ActionData::CustomAction(action).into()
-    }
-
-    #[staticmethod]
-    pub fn value(value: &str) -> Self {
-        accesskit::ActionData::Value(value.into()).into()
-    }
-
-    #[staticmethod]
-    pub fn numeric_value(value: f64) -> Self {
-        accesskit::ActionData::NumericValue(value).into()
-    }
-
-    #[staticmethod]
-    pub fn scroll_target_rect(rect: crate::Rect) -> Self {
-        accesskit::ActionData::ScrollTargetRect(rect.into()).into()
-    }
-
-    #[staticmethod]
-    pub fn scroll_to_point(point: crate::Point) -> Self {
-        accesskit::ActionData::ScrollToPoint(point.into()).into()
-    }
-
-    #[staticmethod]
-    pub fn set_scroll_offset(offset: crate::Point) -> Self {
-        accesskit::ActionData::SetScrollOffset(offset.into()).into()
-    }
-
-    #[staticmethod]
-    pub fn set_text_selection(selection: TextSelection) -> Self {
-        accesskit::ActionData::SetTextSelection(selection.into()).into()
-    }
-}
-
-impl From<accesskit::ActionData> for ActionData {
-    fn from(data: accesskit::ActionData) -> Self {
-        Self(data)
-    }
-}
-
-#[pyclass(get_all, set_all, module = "accesskit")]
+#[derive(Clone)]
+#[pyclass(module = "accesskit", rename_all = "SCREAMING_SNAKE_CASE")]
+pub enum ActionDataKind {
+    CustomAction,
+    Value,
+    NumericValue,
+    ScrollTargetRect,
+    ScrollToPoint,
+    SetScrollOffset,
+    SetTextSelection,
+}
+
+#[pyclass(get_all, module = "accesskit")]
 pub struct ActionRequest {
     pub action: accesskit::Action,
     pub target: NodeId,
-    pub data: Option<Py<ActionData>>,
+    pub data: Option<(ActionDataKind, Py<PyAny>)>,
 }
 
 impl From<accesskit::ActionRequest> for ActionRequest {
     fn from(request: accesskit::ActionRequest) -> Self {
         Python::with_gil(|py| Self {
             action: request.action,
             target: request.target.into(),
-            data: request
-                .data
-                .map(|data| Py::new(py, ActionData::from(data)).unwrap()),
+            data: request.data.map(|data| match data {
+                accesskit::ActionData::CustomAction(action) => {
+                    (ActionDataKind::CustomAction, action.into_py(py))
+                }
+                accesskit::ActionData::Value(value) => (ActionDataKind::Value, value.into_py(py)),
+                accesskit::ActionData::NumericValue(value) => {
+                    (ActionDataKind::NumericValue, value.into_py(py))
+                }
+                accesskit::ActionData::ScrollTargetRect(rect) => (
+                    ActionDataKind::ScrollTargetRect,
+                    Rect::from(rect).into_py(py),
+                ),
+                accesskit::ActionData::ScrollToPoint(point) => (
+                    ActionDataKind::ScrollToPoint,
+                    Point::from(point).into_py(py),
+                ),
+                accesskit::ActionData::SetScrollOffset(point) => (
+                    ActionDataKind::SetScrollOffset,
+                    Point::from(point).into_py(py),
+                ),
+                accesskit::ActionData::SetTextSelection(selection) => (
+                    ActionDataKind::SetTextSelection,
+                    TextSelection::from(&selection).into_py(py),
+                ),
+            }),
         })
     }
 }

bindings/python/src/lib.rs

@@ -46,6 +46,8 @@ fn accesskit(py: Python<'_>, m: &PyModule) -> PyResult<()> {
     m.add_class::<NodeBuilder>()?;
     m.add_class::<Tree>()?;
     m.add_class::<TreeUpdate>()?;
+    m.add_class::<ActionDataKind>()?;
+    m.add_class::<ActionRequest>()?;
     m.add_class::<Affine>()?;
     m.add_class::<Point>()?;
     m.add_class::<Rect>()?;

bindings/python/src/macos.rs

@@ -64,16 +64,19 @@ impl Adapter {
         self.0.update_view_focus_state(is_focused).into()
     }
 
-    pub fn view_children(&self) -> isize {
-        self.0.view_children() as _
+    pub fn view_children(&self, py: Python<'_>) -> PyResult<Py<PyCapsule>> {
+        let ptr: isize = self.0.view_children() as _;
+        Ok(PyCapsule::new(py, ptr, None)?.into())
     }
 
-    pub fn focus(&self) -> isize {
-        self.0.focus() as _
+    pub fn focus(&self, py: Python<'_>) -> PyResult<Py<PyCapsule>> {
+        let ptr: isize = self.0.focus() as _;
+        Ok(PyCapsule::new(py, ptr, None)?.into())
     }
 
-    pub fn hit_test(&self, x: f64, y: f64) -> isize {
-        self.0.hit_test(NSPoint::new(x, y)) as _
+    pub fn hit_test(&self, py: Python<'_>, x: f64, y: f64) -> PyResult<Py<PyCapsule>> {
+        let ptr: isize = self.0.hit_test(NSPoint::new(x, y)) as _;
+        Ok(PyCapsule::new(py, ptr, None)?.into())
     }
 }
 

bindings/python/src/windows.rs

@@ -85,7 +85,6 @@ impl Adapter {
     }
 }
 
-/// This class must only be used from the main thread.
 #[pyclass(module = "accesskit.windows", unsendable)]
 pub struct SubclassingAdapter(accesskit_windows::SubclassingAdapter);
 

pyproject.toml

@@ -5,9 +5,6 @@ build-backend = "maturin"
 [tool.maturin]
 manifest-path = "bindings/python/Cargo.toml"
 features = ["pyo3/extension-module"]
-exclude = [
-    "bindings/python/examples/**"
-]
 include = [
     "LICENSE*"
 ]