Re: GtkTreeView Refactoring Considerations [was Re: Private types inside GTK+]

[Tristan Van Berkom <tristanvb openismus com>]

On Mon, 2010-11-01 at 17:37 +0900, Tristan Van Berkom wrote:
> On Mon, 2010-11-01 at 15:15 +0900, Tristan Van Berkom wrote:
> > On Mon, 2010-11-01 at 12:15 +0900, Tristan Van Berkom wrote:
> > > On Sun, 2010-10-31 at 17:45 +0100, Kristian Rietveld wrote:
> > > > On Sun, Oct 31, 2010 at 3:17 PM, Tristan Van Berkom
> > > > <tristanvb openismus com> wrote:
> > > > > Whew, ok I implemented GtkCellArea->render for GtkCellAreaBox for the
> > > > > most part, however I'm still missing the GtkCellRendererState flags ;-)
> > > > >
> > > > > So for this part I was thinking it might make more sense to create
> > > > > a new GtkCellAreaStateFlags type with just per-row states (and add that
> > > > > as an argument to GtkCellArea->render() and also GtkCellArea->event())
> > > > 
> > > > Most of the states in GtkCellRendererState are actually per-row states
> > > > already (selected, focused, etc.) and are toggled by GtkTreeView and
> > > > GtkTreeViewColumn when rendering the cells.  Though one state,
> > > > "sorted", is obviously per-column.  Did you mean to have a new
> > > > GtkCellAreaStateFlags that will have flags per *cell area* and thus
> > > > per *column*?
> > > 
> > > Ah no I meant per row, I was just seeing how treeviewcolumn does
> > > some hackery around the FOCUS flag (i.e. when treeview column
> > > receives the focus flag at render time, it immediately unflags
> > > that and then re-applies the FOCUS bit to the renderer flags
> > > for actual renderers it decided were to be passed the focus
> > > flag).
> > > 
> > > I suppose that the "sorted" flag also spans the whole column
> > > and not a single cell... since a column can be composed of
> > > one cell area (or even potentially more than one, but lets just 
> > > consider one area per column)... I suppose that all of the flags 
> > > defined by GtkCellAreaStateFlags are interesting to pass along 
> > > to the cell area at render/event time.
> > > 
> > > > 
> > > > > and then somehow tidy up the code that in GtkTreeViewColumn is
> > > > > currently:
> > > > >
> > > > >  _gtk_tree_view_column_count_special_cells (tree_column)
> > > > 
> > > > This function is part of implementing the key navigation behavior.  A
> > > > "special" cell is one that is editable or activatable.  The rule is
> > > > that if there's a single special cell in a column, a focus rectangle
> > > > is drawn spanning all cells in that column.  If there is more than one
> > > > than the focus rectangle will be drawn around single cells.  This
> > > > works fine in many cases, but can of course be awkward in a few
> > > > situations, so perhaps we want to make this configurable in the
> > > > future.  The same likely holds for a situation where you have a check
> > > > box cell renderer and text cell renderer next to each other in a
> > > > column:
> > > > 
> > > >    [x] [my label in a text renderer]
> > > > 
> > > > Akin to a GtkCheckButton, you would want the check to toggle when the
> > > > text renderer is clicked.  And it would be natural if the focus
> > > > rectangle spans the check box and the text renderer. But in other
> > > > situations, with different cell renderers, you do not want this.  This
> > > > is also something to think about and improve for the future.
> > > > 
> > > 
> > > Ah ok this clarifies things... I suppose if cell is activatable
> > > or editable then it CAN_FOCUS :)
> > > 
> > > I'll look into this ... I guess the treeview itself will have
> > > to (and already does) I'll look into how cell areas can handle
> > > their own internal focus chaining... probably they have to 
> > > notify the caller (treeview/column) that focus should be passed 
> > > to a leading or following column (or cell area), track the
> > > currently focused cell etc.
> > 
> > Ok I've ironed out some API for focus handling in the cell area.
> > 
> > However there's one problem which I was still unable to come
> > up with a solution for, it's kindof corner case and admittedly
> > it wont be an issue for the initial refactor but for perfectionism's
> > sake would be really nice to get right.
> > 
> > First I'll present the api I've come up with so far:
> > 
> > 
> > /* Grab focus comes with a "direction" parameter to tell
> >  * the area "from where" the focus is coming from, 
> >  * the ->grab_focus() vfunc should be implemented by
> >  * subclasses so that they can set focus on the appropriate
> >  * cell (top/bottom/left or right cell depending on the
> >  * cell areas type of layout and "direction" were moving
> >  * focus from).
> >  */
> > void gtk_cell_area_grab_focus (GtkCellArea        *area,
> > 			       GtkDirectionType    direction);
> > 
> > /* This fires a signal on the GtkCellArea telling the owning
> >  * layout widget that focus should leave the cell for the
> >  * row indicated by "path", and that it should leave in the
> >  * indicated "direction".
> >  *
> >  * GtkCellLayout implementations should fire this signal when
> >  * handling key events for a given row, when there are no more
> >  * focusable cells and a keystroke would indicate a focus change.
> >  *
> >  * Then parent cell layouting widgets can trap this signal and move
> >  * focus along to another adjacent area... or to the same area and
> >  * move the cursor_row to the next treerow.
> >  */
> > void gtk_cell_area_focus_leave (GtkCellArea        *area,
> > 				GtkDirectionType    direction,
> > 				gchar              *path);
> > 
> > 
> > 
> > /* set_can_focus() should be set by implementing classes such as
> >  * GtkCellAreaBox while renderers are added/removed.
> >  */
> > void     gtk_cell_area_set_can_focus (GtkCellArea        *area,
> > 				      gboolean            can_focus);
> > 
> > /* This can be consulted by cell layouting widgets to see if it's
> >  * appropriate to call grab_focus() for this area or not.
> >  */
> > gboolean gtk_cell_area_get_can_focus (GtkCellArea        *area);
> > 
> > /* Set/Get focus cell is mostly an internal thing, the focused cell
> >  * is set by a subclass like GtkCellAreaBox while it handles key
> >  * events that move the focus
> >  */
> > void             gtk_cell_area_set_focus_cell (GtkCellArea   *area,
> > 				             GtkCellRenderer *renderer);
> > 
> > /* Get the focused cell can be accessed by the superclass GtkCellArea
> >  * to activate/start-editing cells that have focus when processing
> >  * a key event that indicates activate/start-editing (i.e. ENTER key).
> >  */
> > GtkCellRenderer *gtk_cell_area_get_focus_cell (GtkCellArea   *area);
> > 
> 
> Gah, I just realized that these apis also have to depend on the
> currently set row-data, since the same cell can be editable
> in one row but not editable in the next (or even visible for
> that matter).
> 
> Need to put a bit more thought into this... I'll come up with
> something further thought out soon.
> 

Ok I have something better for focus ... but now we're into a
whole other world of trouble that I could really use your help
in resolving.

First of all, I'm pasting the focus related portions of the
api below ... this is how I imagine it would be used by the
treeview:

  - When the treeview receives an event on the bin window it needs 
    to do some filtering:
      a.) In the case it's a key event, the treeview needs to know
          what is the focused row (the cursor_row in current treeview
          jargon), and it needs to know what is the focused column,
          in this case it will:

            gtk_cell_area_apply_attributes (focused_area, model, iter,
                                            is_expander, is_expanded);
            gtk_cell_area_event (focused_area, event... flags...);

      b.) In the case that the event is mouse related a few things can
          happen and I'm not entirely sure how it gets handled but
          roughly it looks like:

          1.) If there is a current drag in effect, handle the drag

          2.) If there is some rubber-band selection going on... need
              to treat the rubber-band state updates

          3.) Finally if it's a new event... the treeview needs to find
              the row & column for the position of the event and again
              call gtk_cell_area_apply_attributes() and then
              gtk_cell_area_event().

          Note here that if we want to really make it possible to render
          widgets in a GtkCellArea, we may need some more semantics such
          as:
             - Adding grabs to the mouse pointer
             - Artificially generating enter/leave notifications

          4.) At this point as a result of an event, the GtkCellArea
              which received the event will give focus to one of its
              renderers (one which started editing or was activated).

    - As a result of handling a key event, it can be that a GtkCellArea
      generates a "focus-leave" signal with the treepath for the event
      and the GtkDirectionType, in this case the treeview needs to
      inspect the GtkDirectionType and decide whether to pass focus
      to an adjacent column in the row... or to pass focus on to the
      next or previous row (or to focus out of the bin_window completely
      and pass focus to another widget, or to a treeview column header
      or such).

      To pass focus to another area one would call:
         gtk_cell_area_grab_focus (area, direction)

    - Similarly the treeview upon receiving focus itself would probably
      go ahead and find the first or last visible row/column and again
      call gtk_cell_area_grab_focus (area, direction) with a direction
      of it's choosing (i.e. probably GTK_DIR_RIGHT on the first
      area/column in the first row).

    - Its important to note that focus information on the area such as
      whether the area itself "can-focus" or what cell is currently
      in focus is not automatically updated by calling:
      gtk_cell_area_apply_attributes() on the area for a row of data.

      for this reason I've added gtk_cell_area_update_focus() which
      checks whether the area "can-focus", this is a vfunc implemented
      on the base class but can be derived for a possible GtkCellArea
      which renders cells and also renders widgets (i.e. a GtkCellArea
      may not contain any renderers which "can-focus" but may contain
      a widget which can).

Parts of this api that are largely unaccounted for are mainly to do with
event handling, having gtk_cell_area_event() is IMO the right way to do
things but it's obviously a huge change from how treeview/column does
things now (and it will be painful)... 

Regards,
     -Tristan

Note: The bit I was complaining about for passing focus from
one tabular area to another are not resolved here, but I think
in this case it's alight to live with something a little less
than perfect.

/**
 * gtk_cell_area_grab_focus:
 * @area: a #GtkCellArea
 * @direction: the #GtkDirectionType from which focus came
 *
 * This should be called by the @area's owning layout widget
 * when focus should be passed to @area for a given row data.
 *
 * Note that after applying new attributes for @area that
 * gtk_cell_area_update_focus() should be called and
 * gtk_cell_area_can_focus() should be checked before trying
 * to pass focus to @area.
 *
 * Implementing #GtkCellArea classes should implement this
 * method to receive focus in it's own way particular to
 * how it lays out cells.
 */
void
gtk_cell_area_grab_focus (GtkCellArea      *area,
			  GtkDirectionType  direction)

/**
 * gtk_cell_area_focus_leave:
 * @area: a #GtkCellArea
 * @direction: the #GtkDirectionType in which focus
 *             is to leave @area
 * @path: the current #GtkTreePath string for the 
 *        event which was handled by @area
 *
 * Notifies that focus is to leave @area in the
 * given @direction.
 *
 * This is called by #GtkCellArea implementations upon
 * handling a key event that caused focus to leave the
 * cell. The resulting signal can be handled by the
 * owning layouting widget to decide which new @area
 * to pass focus to and from what @direction. Or to
 * pass focus along to an entirely new data row.
 */
void
gtk_cell_area_focus_leave (GtkCellArea        *area,
			   GtkDirectionType    direction,
			   const gchar        *path)

/**
 * gtk_cell_area_update_focus:
 * @area: a #GtkCellArea
 *
 * Updates focus information on @area for a given 
 * row of data.
 *
 * After calling gtk_cell_area_apply_attributes() to
 * the @area this method should be called to update
 * information about whether the @area can focus and
 * which is the cell currently in focus.
 */
void
gtk_cell_area_update_focus (GtkCellArea *area)

/**
 * gtk_cell_area_set_can_focus:
 * @area: a #GtkCellArea
 * @can_focus: whether @area can receive focus
 *
 * This is generally called from GtkCellArea::update_focus() 
 * implementations to update if the @area can focus after
 * applying new row data attributes.
 */
void
gtk_cell_area_set_can_focus (GtkCellArea *area,
			     gboolean     can_focus)

/**
 * gtk_cell_area_get_can_focus:
 * @area: a #GtkCellArea
 *
 * Returns whether the area can receive keyboard focus,
 * after applying new attributes to @area, 
 * gtk_cell_area_update_focus() needs to be called before
 * calling this method.
 *
 * Returns: whether @area can receive focus.
 */
gboolean
gtk_cell_area_get_can_focus (GtkCellArea *area)

/**
 * gtk_cell_area_set_focus_cell:
 * @area: a #GtkCellArea
 * @focus_cell: the #GtkCellRenderer to give focus to
 *
 * This is generally called from #GtkCellArea implementations
 * either gtk_cell_area_grab_focus() or gtk_cell_area_update_focus()
 * is called. It's also up to the #GtkCellArea implementation
 * to update the focused cell when receiving events from
 * gtk_cell_area_event() appropriately.
 */
void
gtk_cell_area_set_focus_cell (GtkCellArea     *area,
			      GtkCellRenderer *renderer)

/**
 * gtk_cell_area_get_focus_cell:
 * @area: a #GtkCellArea
 *
 * Retrieves the currently focused cell for @area
 *
 * Returns: the currently focused cell in @area.
 */
GtkCellRenderer *
gtk_cell_area_get_focus_cell (GtkCellArea *area)