Re: [hatari-devel] SDL GUI keyboard shortcuts

[Thomas Huth <th.huth@xxxxxx>]

Am Tue, 12 Aug 2014 23:24:37 +0300
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> Hi,
> 
> On tiistai 12 elokuu 2014, Thomas Huth wrote:
> > Eero, I've played now a little bit with the new keyboard GUI
> > control, and I like it, but I also got some comments:
> 
> Any comments on what to do about the file selector?
>
> GUI has no concept of lists or their focus, so adding
> that just for file selector would be a lot of work.

I think adding some special handling there is still the best thing
that we could do. After all, the fileselector code is already
handling SDL event to implement some special actions like
page-up/down and mouse wheel support.

> Other alternative I was thinking was adding file name
> completion.  When user types first letters for a filename,
> GUI will:
> - put to edit line the first file name matching
>   the typed letters so far
> - move the list so that this file name and ones
>   following it are visible (which could be selected
>   by typing more letters)
> To type another name, user needs to pause typing for
> a while.
> 
> Matching should IMHO be case insensitive, so the list
> sorting should be made case insensitive too.

I don't think this is the right way to go since
a) This is likely also quite a lot of work
b) Sounds somewhat non-intuitive to me, so I guess you
   either end up explaining this feature again and again
   or hardly any user will use it
c) This als only works if the user really has a keyboard in front.
   For controlling the GUI with a joystick, this simply does not work.

> Btw. I'm not completely sure whether the current method
> of using space to activate focuced item is the best.  I
> constantly try to use Enter for that, before remembering
> that Enter is used for the default action.  Using Enter
> for activation would also suit "type to match" better
> than space, as file names can contain spaces.
> 
> However, if Enter is used to activate the focused item,
> there's no way to activate the default item (when focus
> is moved away from it).
> 
> Any comments on that?

I also mixed up Enter and Space when I tried to control the GUI
with the keyboard, so I think there is really something that has
to change here.
What about always activating the focused item with Enter (or Space),
and just putting the focus on the default button at the beginning?
Then behaviour is the same for people who do not use the cursor 
keys (Enter activates the default button). And for people who use
the cursor keys, they certainly do not expect the default button to
be handled with Enter anymore.

> > 3) The blue color for the focus is ... hmm, maybe ok ... but
> > wouldn't it fit better to use a lighter shade of grey instead, for
> > example?
> 
> I'm not attached to that color (it's just a place holder),
> but IMHO the current gray scheme is a bit bland.
> 
> I started to move specifying of the SDL GUI colors into
> same place, in preparation for potential later themeing
> for the colors.  I was thinking that if it were configurable
> from a text file, people could experiment with different
> color combinations and we could before release pick nice one
> as default + ship few good ones with Hatari.
> 
> Would you be interested / have time to look into that?

No, since I personally don't need theming support in Hatari.

> And maybe add joystick support to GUI keyboard navigation?
> :-)

I currently do not got my joystick with me (I'm away from home),
so currently I also can not look at this.

> > 1) I get a compiler warning now:
> > 
> > dlgFileSelect.c: In function ‘SDLGui_FileSelect’:
> > dlgFileSelect.c:515:29: warning: cast discards
> > ‘__attribute__((const))’ qualifier from pointer target type
> > [-Wcast-qual] fsdlg[SGFSDLG_TITLE].txt = (char *)title;
> 
> There's no good way to fix text string handling in SDL GUI.
> Because same field is used both for immutable text fields
> and editable text fields, it needs to be "char *", not const.
> 
> However, most of the strings specified within/for SDL GUI code
> (including file selector titles) are literal strings i.e.
> consts.
> 
> As a result, at some point const string contents need to be set
> to non-const struct member when specifying button/text string.
> Doing it in the current place gives the least amount of warnings.

Hmm, sure, but this has never been a big issue in the past. For example,
I wonder why the recent GCC then does not complain at the other
places, like in dlgScreen.c:

 windowdlg[DLGSCRN_RECANIM].txt = "Stop record";

?

> Other alternative would be to dup the string & free it at every
> function exit, but doing it just to silence a warning seems
> icky.  Strings for non-editfields aren't modified.

That sounds ugly. A proper solution would maybe be to use a union for
the txt field instead, something like:

typedef struct
{
  int type;             /* What type of object */
  int flags;            /* Object flags */
  int state;            /* Object state */
  int x, y;             /* The offset to the upper left corner */
  int w, h;             /* Width and height (for scrollbar : height and position) */
  union {
    char *txt;          /* Editable text string */
    const char *ctxt;   /* Constant text string */
  };
}  SGOBJ;

What do you think about that?

 Thomas