Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Wayne Stambaugh]

+1 from me.

On 8/13/2019 5:34 PM, Ian McInerney wrote:
> Jeff,
> 
> This change would not affect the handling of the menu action events, it
> would simply be in charge of updating the UI of the menus/taskbars to
> reflect the current state of the tools. I think the biggest advantage
> for doing it this way is to unify the method by which the state of all
> menus/toolbars is updated. Currently, the CONDITIONAL_MENU handles the
> state update for those menus but the programmer must handle it for all
> other items. When a new item is added to both a menu and a toolbar they
> then have to ensure it is updated properly in both places.
> 
> What this is proposing is to unify all the update logic into something
> that is abstracted away from the programmer. They simply supply the
> enabling condition (as we already do for the CONDITIONAL_MENU), and the
> framework handles it for them. This eliminates the need to have
> SyncToolbars() at all, and the items are updated when wxWidgets wants
> them not when our framework feels it is needed.
> 
> This might remove the need to call Resolve after creating the menus, but
> I haven't experimented with it. It should definitely remove the need to
> call SyncToolbars() after their creation/modification though.
> 
> I do think your suggestion about renaming CONDITIONAL_MENU to
> CONTEXT_MENU is another reason to do this for the ACTION_MENUs, so then
> we have a definite class demarcation between a context menu and a normal
> menu. The context menu would still inherit from the normal menu, but it
> would include the ability to hide the items.
> 
> -Ian
> 
> On Tue, Aug 13, 2019 at 12:18 AM Jeff Young  > wrote:
> 
> I’m not sure what we’re buying with the wxUpdateUIEvents then. 
> Context menus will still need to run the existing Evaluate() stuff
> (for visibility and for the highlight hacks), at which point it
> might as well do the enabling/checking too. And menu-bar menus will
> also still have to run menu dispatch (for the menu-vs.-accelerator
> hack).  I suppose it would allow us to remove some of the GTK hacks
> (such as having to resolve the menus immediately after building them).
> 
> If we do push down the lamda stuff into ACTION_MENU so that it can
> bind to wxUpdateUIEvents, we should move the menu-bar menus to
> ACTION_MENU and then rename CONDITIONAL_MENU to CONTEXT_MENU.
> 
> Cheers,
> Jeff.
> 
> 
>> On 12 Aug 2019, at 23:06, Ian McInerney > > wrote:
>>
>> 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 > > 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
>>> mailto:ian.s.mciner...@ieee.org>>
>>> wrote:
>>>
>>> Fol

Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Jeff Young]

Hi Ian,

Sounds good.  Carry on. ;)

Cheers,
Jeff.

> On 13 Aug 2019, at 22:34, Ian McInerney  wrote:
> 
> Jeff,
> 
> This change would not affect the handling of the menu action events, it would 
> simply be in charge of updating the UI of the menus/taskbars to reflect the 
> current state of the tools. I think the biggest advantage for doing it this 
> way is to unify the method by which the state of all menus/toolbars is 
> updated. Currently, the CONDITIONAL_MENU handles the state update for those 
> menus but the programmer must handle it for all other items. When a new item 
> is added to both a menu and a toolbar they then have to ensure it is updated 
> properly in both places.
> 
> What this is proposing is to unify all the update logic into something that 
> is abstracted away from the programmer. They simply supply the enabling 
> condition (as we already do for the CONDITIONAL_MENU), and the framework 
> handles it for them. This eliminates the need to have SyncToolbars() at all, 
> and the items are updated when wxWidgets wants them not when our framework 
> feels it is needed.
> 
> This might remove the need to call Resolve after creating the menus, but I 
> haven't experimented with it. It should definitely remove the need to call 
> SyncToolbars() after their creation/modification though.
> 
> I do think your suggestion about renaming CONDITIONAL_MENU to CONTEXT_MENU is 
> another reason to do this for the ACTION_MENUs, so then we have a definite 
> class demarcation between a context menu and a normal menu. The context menu 
> would still inherit from the normal menu, but it would include the ability to 
> hide the items.
> 
> -Ian
> 
> On Tue, Aug 13, 2019 at 12:18 AM Jeff Young  > wrote:
> I’m not sure what we’re buying with the wxUpdateUIEvents then.  Context menus 
> will still need to run the existing Evaluate() stuff (for visibility and for 
> the highlight hacks), at which point it might as well do the 
> enabling/checking too. And menu-bar menus will also still have to run menu 
> dispatch (for the menu-vs.-accelerator hack).  I suppose it would allow us to 
> remove some of the GTK hacks (such as having to resolve the menus immediately 
> after building them).
> 
> If we do push down the lamda stuff into ACTION_MENU so that it can bind to 
> wxUpdateUIEvents, we should move the menu-bar menus to ACTION_MENU and then 
> rename CONDITIONAL_MENU to CONTEXT_MENU.
> 
> Cheers,
> Jeff.
> 
> 
>> On 12 Aug 2019, at 23:06, Ian McInerney > > wrote:
>> 
>> 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 > > 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 >> > 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 co

Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Ian McInerney]

Jeff,

This change would not affect the handling of the menu action events, it
would simply be in charge of updating the UI of the menus/taskbars to
reflect the current state of the tools. I think the biggest advantage for
doing it this way is to unify the method by which the state of all
menus/toolbars is updated. Currently, the CONDITIONAL_MENU handles the
state update for those menus but the programmer must handle it for all
other items. When a new item is added to both a menu and a toolbar they
then have to ensure it is updated properly in both places.

What this is proposing is to unify all the update logic into something that
is abstracted away from the programmer. They simply supply the enabling
condition (as we already do for the CONDITIONAL_MENU), and the framework
handles it for them. This eliminates the need to have SyncToolbars() at
all, and the items are updated when wxWidgets wants them not when our
framework feels it is needed.

This might remove the need to call Resolve after creating the menus, but I
haven't experimented with it. It should definitely remove the need to call
SyncToolbars() after their creation/modification though.

I do think your suggestion about renaming CONDITIONAL_MENU to CONTEXT_MENU
is another reason to do this for the ACTION_MENUs, so then we have a
definite class demarcation between a context menu and a normal menu. The
context menu would still inherit from the normal menu, but it would include
the ability to hide the items.

-Ian

On Tue, Aug 13, 2019 at 12:18 AM Jeff Young  wrote:

> I’m not sure what we’re buying with the wxUpdateUIEvents then.  Context
> menus will still need to run the existing Evaluate() stuff (for visibility
> and for the highlight hacks), at which point it might as well do the
> enabling/checking too. And menu-bar menus will also still have to run menu
> dispatch (for the menu-vs.-accelerator hack).  I suppose it would allow us
> to remove some of the GTK hacks (such as having to resolve the menus
> immediately after building them).
>
> If we do push down the lamda stuff into ACTION_MENU so that it can bind to
> wxUpdateUIEvents, we should move the menu-bar menus to ACTION_MENU and then
> rename CONDITIONAL_MENU to CONTEXT_MENU.
>
> Cheers,
> Jeff.
>
>
> On 12 Aug 2019, at 23:06, Ian McInerney  wrote:
>
> 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  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  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 t

Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Jeff Young]

I’m not sure what we’re buying with the wxUpdateUIEvents then.  Context menus 
will still need to run the existing Evaluate() stuff (for visibility and for 
the highlight hacks), at which point it might as well do the enabling/checking 
too. And menu-bar menus will also still have to run menu dispatch (for the 
menu-vs.-accelerator hack).  I suppose it would allow us to remove some of the 
GTK hacks (such as having to resolve the menus immediately after building them).

If we do push down the lamda stuff into ACTION_MENU so that it can bind to 
wxUpdateUIEvents, we should move the menu-bar menus to ACTION_MENU and then 
rename CONDITIONAL_MENU to CONTEXT_MENU.

Cheers,
Jeff.


> On 12 Aug 2019, at 23:06, Ian McInerney  wrote:
> 
> 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  > 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 > > 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 wou

Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Ian McInerney]

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  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  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@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net

Re: [Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Jeff Young]

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  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@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp

___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp

[Kicad-developers] [RFC] Switch from SyncToolbars to wxUpdateUIEvent

[Ian McInerney]

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@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp