Subject: Re: FVWM: HoverIcon for FvwmButtons (Was: animated FvwmButtons)

From: Dominik Vogt <[email-masked]>
Date: Tue, 13 Jul 2004 15:24:58 +0200

On Tue, Jul 13, 2004 at 12:55:44PM +0200, Dominik Vogt wrote:
> On Sat, Jul 10, 2004 at 07:20:10PM +1000, Scott Smedley wrote:
> > > > Ditto. FvwmButtons is quite complex. Consequently, I'm not comfortable
> > > > commiting this patch until it's had wider testing. Maybe you can convince
> > > > some fellow FVWMers to try it out? :)
> > > 
> > > You'd encourage at least one more FVWMer if you'd just commit the
> > > patch.  Backing out patches is easy while having to apply patches
> > > and taking care not to accidentally commit them is a pain :-)
> > 
> > Ok, committed.
> 
> There was a crash when the pointer enters unused button space,
> i.e. slots that are not allocated to any button.  I have fixed
> this, but there may be similar bugs in other parts of
> FvwmButtons.
> 
> Other issues with HoverColorset:
 
>  1 When you click a button, it is drawn in the original colours as
>    long as the mouse button is held.  Shouldn't it also use the
>    HoverColorset?
>    This is because the ButtonPress handler does not take the
>    current HoverButton into account.

Fixed.

>  2 Move the mouse on any button with an action bound to it.  The
>    button is hilighted.  Now press and release a mouse button 
>    ==> The button is no longer hilighted.
>    This is because the ButtonRelease handler does not take the
>    current HoverButton into account.

Fixed.

>  3 Press a mouse button on a button, drag the pointer to another
>    button, then release the mouse button without moving the mouse
>    ==> The button under the pointer is not hilighted.  As soon as
>    the mouse moves it switches to the HoverColorset again.

Fixed.

>  4 When the pointer is on a container or a swallowed application,
>    the border of that container is hilighted.  I think only
>    buttons that actually have an action bound to them should be
>    hilighted (maybe this should be configurable).
>  
>  5 I'm not sure "Hover" is a good name.  I associate all kinds of
>    things with "hovering", but definitely not the current
>    behaviour.  In the menu code we use "Active" for that purpose.

Ciao

Dominik ^_^  ^_^

 --
Dominik Vogt, [email-masked]
Reply-To: [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 14 Jul 2004 11:44:13 +0200

(I'm moving this discussion to fvwm-workers, where it belongs).

On Wed, Jul 14, 2004 at 10:08:24AM +1000, Scott Smedley wrote:
> Hi Dominik, :)
> 
> Thanks for testing out FvwmButtons.
> 
> I've got quite a few minor fixes in my own tree, & I've rearranged
> the code a bit to make it a bit neater - I'll need to merge & checkin
> soon.
> 
> I think an option is best. Any suggestions for a name?

There might be two settings, a global one and a button specific
one:

  *FvwmButtons: UseActiveColorset Always|Never|ActionButtons

where

  Always:

    For all buttons

  ActionButtons (default)

    For all buttons that have an action bound to them (including
    panel buttons)

  Never

    Turn off completely.

And an option

  UseActiveColorset / !UseActiveColorset

that enables or disables the feature for each button individually,
overriding the global settings.

> Good idea.
> One noticable difference I've made is to draw the button using
> Press{Colorset,Icon,Title} when a button has been clicked & instructed
> to wait for a window to popup. (the former behaviour was just to make
> the button relief sunken.) I think this is intuitive, but would
> appreciate other opinions.

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Thu, 15 Jul 2004 01:27:30 +1000

Hi all,

> >    long as the mouse button is held.  Shouldn't it also use the

Currently, the Active{Colorset,Title,Icon} are used. If none are
specified then Colorset, Title & Icon are used. The effect you
describe is achievable by setting PressColorset to the same colorset
as your ActiveColorset (formerly HoverColorset). I'm undecided if
this is the right behaviour ...

> >    hilighted (maybe this should be configurable).

TODO.

> >    behaviour.  In the menu code we use "Active" for that purpose.

Done.

SCoTT. :)
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Scott Smedley <[email-masked]>
Date: Thu, 15 Jul 2004 01:33:04 +1000

> this is the right behaviour ...

I think this is the right behaviour - it's the main reason we have
PressColorset, PressIcon & PressTitle in the first place. It doesn't
make sense to override them with the Active* options.

Things get complicated when no Press* options are specified, but the
current behaviour is still coherent when a button is waiting for a
window to popup (& the active button could be elsewhere).

SCoTT. :)

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 15 Jul 2004 11:37:41 +0200

> window to popup (& the active button could be elsewhere).

Okay.

But with the latest patch, almost all the bugs I fixed a couple of
days are back (which is no big surprise as most or all of the code
I changed is gone).

 1) After clicking on a button it is no longer hilighted.

 2) Press mouse button 1 on a button without releasing it.  It now
    is drawn with PressColorset.
    Press button 2 without moving the mouse to cancel the action.
    ==> the button is drawn using the normal colorset while it
    should use the ActiveColorset.

 3) Press and hold a mouse button over a button.  It gets
    hilighted as it should.  Then move the pointer out of the
    button into another button.
    ==> It is still hilighted and sunken, but should be raised and
    not hilighted.

 4) Same as (3), but move the pointer out of the FvwmButtons
    window.

Would you please consider backing out your latest changes and redo
them without removing the code I wrote?

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Fri, 16 Jul 2004 00:14:38 +1000

Hi Dominik,

>  1) After clicking on a button it is no longer hilighted.

This is what I was talking about (see above). Presumably you have
no PressColorset specified & are using an action like:

Action Exec "xmessage" exec xmessage hello

Maybe you'd like an option to override the Press* options with the
Active* options (what's the point of having Press* options then?).
I suppose an option to override Press* options with Active* options
if-&-only-if no Press* options are specified could be warranted ...

>     should use the ActiveColorset.

Yep. A bug - will fix soon.

>  4) Same as (3), but move the pointer out of the FvwmButtons

3 & 4: The current behavior is as Mikhael requested. See earlier
posts. (Maybe I misinterpreted?)

> them without removing the code I wrote?

With only one minor bug, hopefully I've convinced you this isn't
necessary. Of course, if you still feel that it is necessary I will comply.

In the interests of harmonious future collaboration I'd like to air
a couple of thoughts:

It's a bit frustrating having multiple people working on exactly the
same bit of code. One could argue that this is the nature of distributed
projects. However, I like to think that most of the time this can be
avoided. In this case, it was clear I was working on the FvwmButtons
module - if a similiar situation were to arise in the future, can I
suggest just requesting me to fix bugs &/or make implementation changes?
This would prevent me from effectively undoing your changes which must
be equally frustrating for you.

Upon reflection, maybe I caused this problem by not checking in my
bugfixes earlier. By my logic, there were no "showstoppers" (mostly
minor cosmetic bugs) in CVS, so I kept fixes in my own tree for a
couple of days so I could thoroughly test while I added a couple of
other features. Perhaps I should commit fixes as soon as they are
done? Don't know. All suggestions/discussion on the way I work on
FVWM most welcome.

Anyway, I hope this discussion isn't inappropriate. I'm hoping some
clear communication now will enable me to avoid causing futher problems
later on.

SCoTT. :)

---

From: Mikhael Goikhman <[email-masked]>
Date: Thu, 15 Jul 2004 15:01:07 +0000

> posts. (Maybe I misinterpreted?)

Well, I requested that other buttons not to be hilighted as long as the
one is still pressed. :)

I think the current behaviour is ok, the pressed button may be locked.
But I am not against to restore the button 3d-ness if the mouse is
outside. I think the Press* properties are ok even if the mouse is
outside, however I have no strong preference. Just implement it so that
users of the transparent (or otherwise no-relief) FvwmButtons feel well.

Regards,
Mikhael.

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 15 Jul 2004 17:53:37 +0200

> I think the current behaviour is ok, the pressed button may be locked.

Thinking about it, I like the current behaviour best.  It gives
the user visual feedback which button she pressed originally.
With the old code, FvwmButtons indicated that it would do nothing
when the mouse button was released, but gave no hint which button
was pressed first.

Ciao

Dominik ^_^  ^_^

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 15 Jul 2004 18:21:23 +0200

> Action Exec "xmessage" exec xmessage hello

No, I'm not.  When I click on any button, it ends up using the
standard colour set until the mous is moved.  That has nothing to
do with the PressedColorset.

> avoided.

> In this case, it was clear I was working on the FvwmButtons
> module

It wasn't clear to me.

> - if a similiar situation were to arise in the future, can I

Usually I don't bother, but in this case describing all the many
bugs in the old and new code I found would have taken more time
than just fixing them.

And by the way, this is the first time I can rember that a
conflicting check in caused a real problem - in the seven years
that I'm using CVS.

> be equally frustrating for you.

Well, my frustration came from the fact that I spent quite some
thought to get event handling straight, and all that code went
down the drain.  But assuming we leave the button hilighting with
mouse button held as it is, all that complex code is no longer
necessary.

> other features.

> Perhaps I should commit fixes as soon as they are done?

I try to never keep uncommitted patches on my disk at the end of
the day because I tend to forget what I already committed and end
up describing the same change twice (or not at all) in the commit
messages.

If it's necessary to keep changes for a longer period of time, I
just ask on the list to keep the code alone for the time being.

> Don't know. All suggestions/discussion on the way I work on
> FVWM most welcome.

I usually care only about code readability and ease of
maintenance.  My preferences are all written down in
docs/CONVENTIONS, but we never discussed any of these
suggestions.

Currently, I'm particularly unhappy with the level of nesting we
have in some code.  In some files, it's impossible to keep lines
shorter than 80 characters.  I hope that future code will nest no
deeper than 4 levels of indentation.

Another problem that is going to become worse in the future is
code that is correct by the C99 standard but not by C89
(specifically C++ style comments with // and declarations in the
body of code blocks, e.g. { int i; i=1; intj; }).  Is there an
option to tell gcc to adhere to the C89 standard?

> clear communication now will enable me to avoid causing futher problems

You're not causing trouble, you're doing very valuable work.  This
is what keeps fvwm alive.

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Fri, 16 Jul 2004 10:28:43 +1000

Hi Dominik,

> When I click on any button, it ends up using the

Ok, this must be a bug - I will check/fix tonight.

> option to tell gcc to adhere to the C89 standard?

gcc -std=c89 ... 

This works for me. (I'm using gcc 3.2.2) It detects both '//' &
loop declarations & reports them as errors.

Very handy, as I'm sure I would otherwise let '//' comments slip
through ocassionally. It's too ingrained! :)

SCoTT. :)

---

From: Dominik Vogt <[email-masked]>
Date: Fri, 16 Jul 2004 11:54:39 +0200

> through ocassionally. It's too ingrained! :)

Unfortunately, my gcc (2.95.4) fails to compile some of its own
header files with -std=c89, e.g. sys/ipc.h :-P

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Fri, 16 Jul 2004 23:25:52 +1000

Hi Dominik, :)

> > do with the PressedColorset.

I can't reproduce this.

I'm using an ActiveColorset, doing a buttonpress on a button which gets
displayed using the standard colorset. Now I wiggle the mouse all over
the place but nothing changes until I release the mouse button.

Can I trouble you for a test config?

SCoTT. :)

---

From: Dominik Vogt <[email-masked]>
Date: Mon, 19 Jul 2004 09:50:49 +0200

> the place but nothing changes until I release the mouse button.

You misunderstood me.  In short, when I said "click" I meant press
+ release.  Try this:

  - Use an ActiveColorset.  It does not matter whether you use the
    PressedColorset too.
  - Move the mouse over a pressable button to hilight it.  Don't
    move any more during this test.
  - Press the mouse button.
  - Release the mouse button.

 ==> The button is no longer hilighted

  - Move the mouse.

 ==> The button gets hilighted again.

To fix this, either the ButtonRelease or the
EnterNotify/LeaveNotify handlers (or both) have to be
ActiveColorset aware.

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Wed, 21 Jul 2004 22:03:38 +1000

Hi Dominik, :)

>  ==> The button gets hilighted again.

I can't reproduce this either. The button is highlighted just fine
after I release the mouse button (without moving the mouse).

I know this was a bug in the first version I committed to CVS. I know
it's (very!) unlikely, but I don't suppose you're running an older version? :)

SCoTT. :)

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 21 Jul 2004 16:13:01 +0200

> it's (very!) unlikely, but I don't suppose you're running an older version? :)

It seems to happen only if a function is called, not with plain
commands.  Try

  DestroyFunc foo
  AddToFunc foo I nop
  DestroyModuleConfig FvwmButtons*
  *FvwmButtons: ActiveColorset 2
  *FvwmButtons(Title foo, Action foo)
  Module FvwmButtons

Ciao

Dominik ^_^  ^_^

---

From: Scott Smedley <[email-masked]>
Date: Fri, 23 Jul 2004 17:13:13 +1000

Hi Dominik, :)

>   *FvwmButtons(Title foo, Action foo)

This is a bizarre bug!

The button is actually redrawn (correctly) with the active colorset, but
the FvwmButtons module immediately receives a LeaveNotify event (I don't
move the mouse) & redraws the button with the standard colorset
(thinking that the mouse cursor is no longer in the FvwmButtons window).

You can also do:

sleep 6 && FvwmCommand foo

whip the mouse into _any_ button & wait for the LeaveNotify event
to get triggered (causing the active button to be drawn in the standard
colorset, instead of the active one).

So, why does invoking function "foo" cause FvwmButtons to receive an
erroneous LeaveNotify event?

SCoTT. :)

---

From: Dominik Vogt <[email-masked]>
Date: Tue, 27 Jul 2004 11:56:25 +0200

> erroneous LeaveNotify event?

I don't think the event is wrong.  When a complex function is
executed, fvwm grabs the pointer.  This automatically generates a
LeaveNotify event for the window that contained the pointer and an
EnterNotify EnterNotify for the window that grabbed the button.
When the grab is released, the opposite events are generated.

FvwmButtons handles LeaveNotify events only, so the final
LeaveNotify unhilights the button.  I've committed a fix.

Also, drawing of panel buttons was wrong during the panel
animation (fixed too).

Finally, there is still the issue that empty buttons that are
waiting for their application to be spawned are drawn in the
PressColorset.  This may be good for buttons that spawn their
application with a click, but is ugly for all other buttons
because of the flashing at startup.  I've made a patch that uses
said behaviour only for buttons with the UseOld flag set, but I
think it should be removed completely.  If such a feature is
worthwhile it use an entirely separate colour set (SwallowColorset
or WaitColorset or ...).

Ciao

Dominik ^_^  ^_^