Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request]: Change the color button from automatically launching the dialog, and I can respond to an event to instead open my own dialog? #396

Closed
Smurf-IV opened this issue Sep 28, 2021 · 32 comments
Labels
completed This issue has been completed. enhancement New feature or request new feature A new feature has been requested. suggestion A suggestion has been requested.
Milestone

Comments

@Smurf-IV
Copy link
Member

Smurf-IV commented Sep 28, 2021

#392

#392 (comment)

@Smurf-IV Smurf-IV added enhancement New feature or request new feature A new feature has been requested. suggestion A suggestion has been requested. labels Sep 28, 2021
@Smurf-IV Smurf-IV self-assigned this Oct 4, 2021
@Smurf-IV Smurf-IV added the under investigation This bug/issue is currently under investigation. label Oct 4, 2021
@Smurf-IV
Copy link
Member Author

Smurf-IV commented Oct 4, 2021

I'll check this in tomorrow:
ClickOverride

@sdreb3421
Copy link

To simplify things on my end I'll likely try to inherit from KryptonColorButton to automatically launch a custom dialog. Will there be a new click method or event to override?

@Smurf-IV
Copy link
Member Author

Smurf-IV commented Oct 5, 2021

In the above, Just add a click event, and it calls that instead of the default drop / splitter menu thingy.
It also removes the Splitter and Dropdown drawing from the button at runtime (Not in the designer - as it does not know about events!)

So if you derive from it (For whatever reason) then yes it will still do it's job correctly.

@sdreb3421
Copy link

So the presence of a click event listener is what turns off the automatic dialog? That works for me, but others may have a unique reason to handle a click event but still expect the auto dialog. Either way, looking good

@Smurf-IV
Copy link
Member Author

Smurf-IV commented Oct 5, 2021

So the presence of a click event listener is what turns off the automatic dialog?

If they have a reason, then they can also call the existing function(s) that performed the drop / menu stuff, before the their existing click code.

So yes. It is a breaking change, but I feel that this has the least impact, but with the most sensible gain.

@sdreb3421
Copy link

I just noticed that I can't subscribe to the click event from within a derived class. But I can override the OnClick method. Will this also prevent the automatic dialog? I'm trying to avoid adding an event handler for every color button in the application just to launch the same dialog. Thanks

@Smurf-IV
Copy link
Member Author

Smurf-IV commented Oct 5, 2021

Just a thought, but in the Derived class constructor,
just do Click += func;

Smurf-IV added a commit to Krypton-Suite/Standard-Toolkit-Demos that referenced this issue Oct 5, 2021
Smurf-IV added a commit that referenced this issue Oct 5, 2021
…untime

- Change button to also remove splitter and drop down arrow
Implements #396
@Smurf-IV Smurf-IV removed their assignment Oct 5, 2021
@Smurf-IV Smurf-IV added completed This issue has been completed. and removed under investigation This bug/issue is currently under investigation. labels Oct 5, 2021
@Smurf-IV Smurf-IV closed this as completed Oct 5, 2021
@sdreb3421
Copy link

@Smurf-IV I implemented this change back in October using the Nightly build and everything worked (subscribed to click event in derived constructor). But it no longer works in latest Nightly or stable build (6.2111.312). The original color popup appears and the click event doesn't get called. Would you check and see if it got stripped out somewhere? Thanks

@Smurf-IV
Copy link
Member Author

@Wagnerp ??

@Smurf-IV Smurf-IV reopened this Nov 11, 2021
@PWagner1
Copy link
Contributor

@Wagnerp ??

@Smurf-IV I remember changing it from the Windows colour dialog to the Krypton one. @sdreb3421 Is this what you mean?

@sdreb3421
Copy link

@Smurf-IV changed the code so that subscribing to the click event would prevent the standard pop up from showing. This allowed me to open my own dialog.

@sdreb3421
Copy link

Do you need any additional info? Is this fix going to be resolved in an upcoming release?

@PWagner1
Copy link
Contributor

Do you need any additional info? Is this fix going to be resolved in an upcoming release?

If you can provide any additional info, that would be great!

@sdreb3421
Copy link

The code should be reupdated to disable the normal colorbutton pop up if the click event is subscribed to. The original reason for this fix was the new kryptoncolordialog does not properly support larger fonts. This allows us to use our own color dialog. Thank you!

@PWagner1 PWagner1 self-assigned this Nov 17, 2021
@PWagner1 PWagner1 added this to the January 2022 milestone Nov 17, 2021
@PWagner1
Copy link
Contributor

Adding a property (bool) to allow the usage of the Windows CD over the Krypton version

@sdreb3421
Copy link

A property would be great, but could there be an option to have no dialog?

@PWagner1
Copy link
Contributor

A property would be great, but could there be an option to have no dialog?

@Smurf-IV Is this a possibility?

@Smurf-IV
Copy link
Member Author

If the Override worked as before (i.e. detect if the callback had been set) then it is upt o the owner of that callback to perform whatever action they require.
No need for extra properties or fields.

@PWagner1
Copy link
Contributor

If the Override worked as before (i.e. detect if the callback had been set) then it is upt o the owner of that callback to perform whatever action they require. No need for extra properties or fields.

I don't understand, as the implemented override code is still in there, but @sdreb3421 says it doesn't work, even though it worked before?

@sdreb3421
Copy link

What's the plan for this one? Are you guys able to verify the issue on your end? I have these color buttons in many places so need a direction to go before a customer release in January. Thanks

@PWagner1 PWagner1 removed their assignment Nov 23, 2021
@PWagner1
Copy link
Contributor

What's the plan for this one? Are you guys able to verify the issue on your end? I have these color buttons in many places so need a direction to go before a customer release in January. Thanks

@sdreb3421 Are you able to reuse the code in this demo. Nothing has changed with the feature since PR #421.

@sdreb3421
Copy link

What do you mean reuse? How do I download the demo?

@PWagner1
Copy link
Contributor

What do you mean reuse? How do I download the demo?

i.e. override it myColorButton.Click += Click;. Have a look at the color button examples in the latest demos.

@sdreb3421
Copy link

I am currently overriding it in my code, but still get the standard color popup. Are you seeing this as well in the latest stable release?

@PWagner1
Copy link
Contributor

It works in the latest canary/nightly build, not scheduled to enter the stable build 2201 until January.

Animation

@PWagner1
Copy link
Contributor

@sdreb3421 Which version was it working in, so I can compare the files?

@PWagner1
Copy link
Contributor

@sdreb3421 & @Smurf-IV According to WinMerge, the files in both PR #421 & the latest nightly are identical. Is there something missing?

@PWagner1
Copy link
Contributor

@Smurf-IV What does _clickOverriden do?

@PWagner1
Copy link
Contributor

Something is wrong here:

 private void OnButtonClick(object sender, MouseEventArgs e)
        {
            var showingContextMenu = false;

            // Do we need to show a drop down menu?
            if (!_clickOverriden
                && (!Splitter || (Splitter && _drawButton.SplitRectangle.Contains(e.Location)))
                )
            {
                showingContextMenu = ShowDropDown();
            }
            else
            {
                // Raise the standard click event
                OnClick(EventArgs.Empty);

                // Raise event to indicate it was a mouse activated click
                OnMouseClick(e);
            }

            // If not showing a context menu then perform cleanup straight away
            if (!showingContextMenu)
            {
                ContextMenuClosed();
            }
        }

I don't know what yet?

@PWagner1
Copy link
Contributor

@Smurf-IV Could the trick in PR #475 work here?

@PWagner1
Copy link
Contributor

@sdreb3421 & @Smurf-IV Found the issue, this is not currently, in the stable release, as this feature is pegged for build 2201. Therefore, I'm closing this thread. Tested and verified as working on the latest canary/nightly builds.

@sdreb3421
Copy link

Great, thanks guys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This issue has been completed. enhancement New feature or request new feature A new feature has been requested. suggestion A suggestion has been requested.
Projects
None yet
Development

No branches or pull requests

3 participants