native_context_menu icon indicating copy to clipboard operation
native_context_menu copied to clipboard

Disabled Menu Item

Open sanihaq opened this issue 3 years ago • 2 comments

There should be a disabled state for menu item when no function is passed.

sanihaq avatar Nov 30 '21 13:11 sanihaq

FWIW, I needed this recently so added it in my local fork with this commit: https://github.com/edwardaux/native_context_menu/commit/3c2f5c43aae702ae35756e944dc3e805aca98cc8

A couple of important notes:

  • There's an onSelected function on MenuItem that doesn't seem like it is used at the moment. It was a bit unclear what the difference was designed to be between this callback and onItemSelected in ContextMenuRegion. My assumption is that I'd use the former if I wanted different functions to be called back for each item, and I'd use the latter if I just wanted to have one function (and if both were present, then both would be called). However, as I mentioned, MenuItem.onSelected isn't currently hooked up, so I had to add a call to ContextMenuRegion to make sure it calls both.
  • For my personal needs, I adopted the Flutter convention of disabling the widget if the onSelected function is not set. However, this would be a breaking change for any current users of this library (all menu items would be disabled unless they were setting the onSelected in their MenuItem objects - which they wouldn't be because those callbacks don't currently get called)
  • I'm happy to raise a PR back here if @lesnitsky would like, however a) I think the caveats above may mean that it is a breaking change, and b) I don't have a windows or a linux machine to be able to do a similar thing.

In any event, I just thought I'd share in case anyone else needed to do something similar.

edwardaux avatar Apr 15 '22 21:04 edwardaux

please do a PR

I'm completely ok with the breaking change (that will mean a major version bump, so existing users are not affected unless upgrade the version)

the unused callback is likely a child of bad refactor

On Sat, Apr 16, 2022, 12:03 AM Craig Edwards @.***> wrote:

FWIW, I needed this recently so added it in my local fork with this commit: @.*** https://github.com/edwardaux/native_context_menu/commit/3c2f5c43aae702ae35756e944dc3e805aca98cc8

A couple of important notes:

  • There's an onSelected function on MenuItem that doesn't seem like it is used at the moment. It was a bit unclear what the difference was designed to be between this callback and onItemSelected in ContextMenuRegion. My assumption is that I'd use the former if I wanted different functions to be called back for each item, and I'd use the latter if I just wanted to have one function (and if both were present, then both would be called). However, as I mentioned, MenuItem.onSelected isn't currently hooked up, so I had to add a call to ContextMenuRegion to make sure it calls both.
  • For my personal needs, I adopted the Flutter convention of disabling the widget if the onSelected function is not set. However, this would be a breaking change for any current users of this library (all menu items would be disabled unless they were setting the onSelected in their MenuItem objects - which they wouldn't be because those callbacks don't currently get called)
  • I'm happy to raise a PR back here if @lesnitsky https://github.com/lesnitsky would like, however a) I think the caveats above may mean that it is a breaking change, and b) I don't have a windows or a linux machine to be able to do a similar thing.

In any event, I just thought I'd share in case anyone else needed to do something similar.

— Reply to this email directly, view it on GitHub https://github.com/lesnitsky/native_context_menu/issues/11#issuecomment-1100393394, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPYUNUIN34GSWTMR3HPXPLVFHKRBANCNFSM5JB4C7DA . You are receiving this because you were mentioned.Message ID: @.***>

lesnitsky avatar Apr 15 '22 21:04 lesnitsky