[PATCH wayland-protocols 3/3] xdg-shell: Introduce xdg_tooltip

[Mike Blumenkrantz]

Hi,

I have some suggestions which I've inlined below:

On Tue, 12 Jan 2016 16:16:49 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> An xdg_tooltip is a new window type used to implement tooltip like
> surfaces. See the interface documentation for details.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 47 ++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index 276d9fc..91f657a 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -183,6 +183,23 @@
>        <arg name="y" type="int"/>
>      </request>
>  
> +    <request name="get_tooltip">
> +      <description summary="assign the xdg_tooltip surface role">
> +	This creates an xdg_tooltip for the given xdg_surface and gives the
> +	associated wl_surface the xdg_tooltip role. A wl_surface can only have
> +	one xdg_tooltip role. If the wl_surface is given the xdg_tooltip role
> +	while it already has an active xdg_tooltip role, or if it has been given
> +	any other role before, an error is raised.

I think my comment on the first patch proves its relevance here as this section could
be greatly shortened by specifying singular surface role semantics in xdg_surface.

> +
> +	See the documentation of xdg_tooltip for more details about what an
> +	xdg_tooltip is and how it is used.
> +      </description>
> +      <arg name="id" type="new_id" interface="zxdg_tooltip_v6"/>
> +      <arg name="parent" type="object" interface="zxdg_surface_v6"/>
> +      <arg name="x" type="int"/>
> +      <arg name="y" type="int"/>
> +    </request>
> +
>      <request name="set_window_geometry">
>        <description summary="set the new window geometry">
>  	The window geometry of a surface is its "visible bounds" from the
> @@ -666,4 +683,34 @@
>  
>    </interface>
>  
> +  <interface name="zxdg_tooltip_v6">
> +    <description summary="tooltip surface">
> +      This interface defines an xdg_tooltip role that provides functionality
> +      related to tooltip like surfaces.
> +
> +      An xdg_tooltip is temporary a surface that is part of another xdg_surface
> +      (such as xdg_toplevel or xdg_popup) such as a tooltip above a UI widget. It
> +      will always be mapped above both its parent and if the parent has a
> +      xdg_popup child it will also be mapped above that and all other possible
> +      chained xdg_popup surfaces.

"An xdg_tooltip is a temporary surface which is displayed over its parent xdg_surface.
The last-created xdg_tooltip surface will always be the top-most child of the parent xdg_surface."

> +
> +      The parent surface must either have the surface role xdg_toplevel,
> +      xdg_popup or xdg_tooltip.
> +
> +      Being different from xdg_popup, it does not take an active grab while
> +      being mapped, and it will never be automatically dismissed by any
> +      predefined user interaction. The client must itself unmap it using the
> +      xdg_tooltip.destroy request.

Wouldn't it be enough to have

"An xdg_tooltip does not take an active grab while mapped. xdg_tooltip surfaces are
either directly unmapped by clients using the xdg_tooltip.destroy request or indirectly
unmapped when their parent surface is unmapped."

> +
> +      An xdg_tooltip can receive input assuming it has an input region.

"An xdg_tooltip obeys normal input region semantics." or similar ?

> +
> +      If for some reason its parent is unmapped, for example if the parent is a
> +      popup being dismissed, the tooltip will be unmapped as well.

I think this is now covered a few lines up?

> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      Unmap the tooltip surface and destroy the object.
> +    </request>
> +  </interface>
> +
>  </protocol>