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

Close flyout when clicking outside of it #965

Closed
wants to merge 65 commits into from
Closed

Close flyout when clicking outside of it #965

wants to merge 65 commits into from

Conversation

zeluisping
Copy link
Contributor

PreviewMouseDown on MetroWindow which tries to find a Flyout parent, if it does not exist, then close the open flyouts.

PreviewMouseDown on flyoutModal to close modal flyouts when clicking on the darken area.

Fix_es #962
Should not be merged: #1000

PreviewMouseDown on MetroWindow which tries to find a Flyout parent, if it does not exist, then close the open flyouts.

PreviewMouseDown on flyoutModal to close modal flyouts when clicking on the darken area.
@AzureKitsune
Copy link
Member

Could we add a property for enabling this feature? We don't want to break things.

@zeluisping
Copy link
Contributor Author

Yup, no problem.

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

I'm not sure if this should be the default behavior, how about a flag in the Flyout control class that indicates if the flyout should be automatically closed?

Also this implementation currently responds to a left or right click, maybe we should make this customizable, so the developer can select if the flyout should handle the left, right or any mousebutton

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

@Amrykid You beat me to it

@zeluisping
Copy link
Contributor Author

@flagbug Yeah, totally forgot about that.

@topvis
Copy link

topvis commented Jan 20, 2014

Just curious, is IsPinnable property originally reserved for this purpose?

@AzureKitsune
Copy link
Member

.... Probably. That's a good idea.

Added properties for closing on mouse click.
Removed CloseOnExternalClick.
Renamed ExternalClickMouseButton to NotPinnableCloseButton.
@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

Some ideas: Rename IsPinnable to IsPinned and make the default value true
That's a breaking change but IsPinnable didn't even had a use, so it should be fine

The name NotPinnableCloseButton is a bit confusing, we should find a better name for this, maybe ExternalCloseButton ?

@topvis
Copy link

topvis commented Jan 20, 2014

I was guessing if IsPinnable was reserved for this purpose. But I would prefer to use a new property IsAutoClose. IsPinnable by name shows whether the flyout has the capability of being pinned or not, not necessisarily pinned or not.

@topvis
Copy link

topvis commented Jan 20, 2014

@flagbug same idea here. IsPinned (or IsAutoClose) is better than IsPinnable.

Renamed IsPinnable to IsPinned and set it's default value to true.
Renamed NotPinnableCloseButton to ExternalCloseButton.
Added the ability to customize the mouse button that hides flyouts when Flyout::IsPinned is false.
Added OverrideIsPinned to be able to make all flyouts behave as if IsPinned is set to false (even if it is set to true).
Added functionality for FlyoutsControl::OverrideIsPinned, which makes all Flyouts behave as if IsPinned is set to false.
@zeluisping
Copy link
Contributor Author

@flagbug It is now ready for a review.

I also added the property OverrideIsPinned to FlyoutsControl that makes all flyouts behave as if IsPinned is set to false (with default value of false).

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

🆒 can you add some descriptive XML docs for the properties respectively update the doc for IsPinned ?

public static readonly DependencyProperty OverrideIsPinnedProperty =
DependencyProperty.Register("OverrideIsPinned", typeof(bool), typeof(FlyoutsControl), new PropertyMetadata(false));

public MouseButton ExternalCloseButton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this the same as IsPinned?

ExternalCloseButton as property for each individual flyout and OverrideExternalCloseButton for FlyoutControl?

@zeluisping
Copy link
Contributor Author

@flagbug Yes, totally, I'm just going to eat lunch real quick.

@topvis topvis mentioned this pull request Jan 20, 2014
Added ExternalCloseButton and updated documentation.
Replaced ExternalCloseButton with OverrideExternalCloseButton.

Added documentation to OverrideIsPinned and OverrideExternalCloseButton.
Added implementation for FlyoutsControl::OverrideExternalCloseButton and Flyout::ExternalCloseButton.
@zeluisping
Copy link
Contributor Author

@flagbug It's ready for another review.

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

Looks good 👍

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

Cool, so if there aren't any objections, I'm gonna merge this

@flagbug
Copy link
Member

flagbug commented Jan 20, 2014

Ah wait, our development model is a bit stupid. Have to wait for this till 0.12.1 is built

@zeluisping
Copy link
Contributor Author

#1000 Replaces this.

@zeluisping zeluisping closed this Feb 3, 2014
@zeluisping zeluisping deleted the patch-4 branch February 3, 2014 21:33
AzureKitsune added a commit that referenced this pull request Feb 4, 2014
Close Flyouts with external click (makes #965 obsolete)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.