kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

Jeff,

I was thinking we would only move the update for the enable and check
attributes to the wxUpdateUIEvent mechanism and leave the visibility to the
conditional menu to deal with in its Evaluate() function as it currently
does. There is no way to get wxWidgets to hide items for us in menus (and
GTK doesn't even seem to have a mechanism for it, so it will probably never
be possible to outsource it to wxWidgets), so we will always need to handle
that ourselves and the Evaluate() command seems to be doing a decent job of
it.

-Ian

On Mon, Aug 12, 2019 at 10:19 PM Jeff Young <jeff@xxxxxxxxx> wrote:

> Hi Ian,
>
> It sounds promising, but there are some potential pitfalls.  wxWidgets
> only supports two bits: enabled and checked.  CONDITIONAL_MENU supports
> 2-1/2: the enabled flag means visible if the is_context_menu flag is set.
> This is probably a subtlety that does us no good: it would be more direct
> to just have ACTION_MENU support 3 bits (visible, enabled and checked) and
> get rid of CONDITIONAL_MENU.
>
> However, that then begs the question of how do we handle the visible bit
> through the wxUpdateUIEvent mechanism?  Maybe we just use the
> wxUpdateUIEvent as a trigger to run the equivalent of Evaluate()?  Or maybe
> we just have wxUpdateUIEvent handle the two bits, and continue to handle
> the visible bit through the existing mechanism?  (Note that at least some
> of the existing mechanism has to stay anyway because we use it to determine
> if a command is an accelerator or a menu pick.)
>
> Generally speaking we *could* move everything to ACTIONs.  The stuff
> that’s still in the old system at present is just the stuff that didn’t
> look worth moving.  So if there’s anything that needs synchronizing that
> isn’t currently an ACTION we should just make it an ACTION.  (There are two
> caveats to this due to wxWidgets crankiness: Quit and Close.  But neither
> of them need synchronizing.)
>
> Cheers,
> Jeff.
>
>
> On 12 Aug 2019, at 20:45, Ian McInerney <Ian.S.McInerney@xxxxxxxx> wrote:
>
> Following on from the discussion in this merge request (
> https://code.launchpad.net/~imcinerney/kicad/+git/kicad/+merge/371156), I
> thought a little about if the current framework could be adapted to use the
> wxUpdateUIEvent handlers to do the synchronization, and I think it can be.
> Here is my thinking of how it can be done, and I would appreciate comments
> about this idea.
>
> From my understanding there are three classes that will create
> menu/toolbar items that need to be synced: ACTION_MENU, ACTION_TOOLBAR, and
> CONDITIONAL_MENU. The first change would be to make the Add functions for
> all of these classes take a SelectionConditions argument that will be used
> to define the enable/checkmark status of the item (currently only
> CONDITIONAL_MENU takes these).
>
> First question, do we need to synchronize any items that aren't
> action-based? If we will only synchronize action-based items, then only
> those Add functions would need to be extended.
>
> The TOOL_MANAGER would then handle the majority of the work. The Add
> functions would call into the TOOL_MANAGER requesting an event handler be
> created, passing the selection conditions as well.
>
> To create the event handler, the TOOL_MANAGER would do the following:
> 1) Consult a list that associates actions with their selection conditions
> (being on the list would indicate the action already has a handler in the
> window containing the tool manager).
> 2) If the action is on the list, compare the provided selection condition
> with what is already in the list, and if they are different from each
> other, assert (this will require selection conditions used in multiple
> places in the same tool manager for the same action to point to the same
> object). This way the code is kept synchronized as well.
> 3) If it is not on the list, then call Bind and bind an event handler for
> the event. The selection condition would be passed into the Bind as the
> user data so that it can be used in the handler. This handler would be
> bound to the parent of the tool manager.
>
> There would be two handlers, one for checkmark entries and one for
> enable/disable entries, and the correct one would be bound based on the
> type of menu item. These would modify the aEvent object to actually
> check/enable/disable the UI element (like what the original handlers did).
>
> I believe this will remove the need for the SyncToolbars functions (and
> the need for the tool manager to explicitly call this, which had caused
> some issues in the past if I remember correctly). It will also mean that
> the conditional menu's Evaluate function would no longer need to do the
> checking/enabling itself, since that is handled in the event handler.
>
> Thoughts?
>
> -Ian
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
>