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] Developers should be able to provide custom AlphaMasks to DropShadowPanel for their controls #3693

Closed
arcadiogarcia opened this issue Jan 27, 2021 · 7 comments
Labels
controls 🎛️ feature request 📬 A request for new changes to improve functionality in progress 🚧
Milestone

Comments

@arcadiogarcia
Copy link
Member

arcadiogarcia commented Jan 27, 2021

Describe the problem this feature would solve

Currently it is impossible to use DropShadowPanel to project masked shadows for custom usercontrols, since GetAlphaMask is only implemented in a handful of framework controls (Image, Shape, TextBlock) the DropShadowPanel only attempts to get an alpha mask if the Content is an instance of those types.

This is especially painful if you need to wrap all your Images/Shapes/Text in some other control, since ideally you would just "pass through" the alpha mask provided by the native control.

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/0d43da7ae459d000ee29dcb108cc08a56bf5073f/Microsoft.Toolkit.Uwp.UI.Controls/DropShadowPanel/DropShadowPanel.cs#L171

Describe the solution

Solution A:

To allow developers to leverage DropShadowPanel for their own controls, we could define an interface e.g.

interface IAlphaMask
{
     CompositionBrush GetAlphaMask();
}

that they can implement to specify which alpha mask they want to be used, and DropShadowPanel would recognize it and use the right mask.

Solution B:

DropShadowPanel could have a SetContentAlphaMask method that allows the developer to provide alpha masks. This alpha mask would override the one that is provided by the content control, if it has one. This would be very useful to create non-rectangular shadows (e.g. with round corners) for existing framework elements (e.g. grid, inkcanvas...).

I think both solutions would be worth having in the control, I have been in many situations where I would have used them if they were available.

I'm happy to contribute this if it sounds valuable :)

@arcadiogarcia arcadiogarcia added the feature request 📬 A request for new changes to improve functionality label Jan 27, 2021
@ghost
Copy link

ghost commented Jan 27, 2021

Hello, 'arcadiogarcia! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

@arcadiogarcia arcadiogarcia changed the title [Feature] DropShadowPanel [Feature] Developers should be able to provide custom AlphaMasks to DropShadowPanel for their controls Jan 27, 2021
@Kyaa-dost
Copy link
Contributor

@arcadiogarcia Thanks for the detailed info and for taking an initiative on this feature. I am going to open this up for discussion and see what other community members think and then you can proceed to open the PR 🚀

@michael-hawker
Copy link
Member

Thanks @arcadiogarcia we definitely want to make some improvements to this control, it's just fallen out-of-scope for 7.0. I believe @JustinXinLiu may have a fork he's done some work on too (which may overlap), so we should all coordinate. 🙂

Part of what I was going to look at is inverting the relationship so it's not a panel anymore, so I'll definitely make sure to expose it in anything new. But there could still be improvements we could make in the meantime until that new thing is put together (if it works that is).

@JustinXinLiu any details you can provide us, or is your code in a fork somewhere? Maybe we can use that as a starting point for further improvements from @arcadiogarcia at least?

@michael-hawker
Copy link
Member

(Sorry, hit the wrong button by mistake...)

@arcadiogarcia
Copy link
Member Author

This is the fork I've been using myself; I'd be happy to contribute it as a temporary solution: #4107

@arcadiogarcia
Copy link
Member Author

Closing this now that the PR has been merged :)

@michael-hawker
Copy link
Member

I was leaving it open so we remember to add support for this on our new shadows too 😉

@ghost ghost added the in progress 🚧 label Aug 12, 2021
ghost pushed a commit that referenced this issue Aug 27, 2021
## Fixes #3122 #3607 #3516

_Also implements #3693 for the new DropShadow._

FYI @seanocali as this is a different implementation approach (which is simpler to use outside of the DropShadowPanel we've been working on) but should hopefully achieve the same result.

This PR adds attached shadows which can easily be attached to any FrameworkElement without needing to modify the layout like DropShadowPanel does today. They can also be shared using a resource, added to the style of an element, and animated! All the things! 🎉

## PR Type

What kind of change does this PR introduce?

<!-- Please uncomment one or more options below that apply to this PR. -->

<!-- - Bugfix -->
- Feature by @Ryken100 and integrated/extended by @michael-hawker 
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->

## What is the current behavior?

DropShadowPanel is clunky and requires modifying how you layout your app.

## What is the new behavior?

Just attach a shadow and be done! (DropShadowPanel is deprecated.)

## PR Checklist

- [x] Composition Only Shadow Support? (with Target)
- [x] Add XML Docs
- [x] Animation Support to Explicit Animation System?
- [x] Bug can't use `AttachedCardShadow` directly with `Border`?

Please check if your PR fulfills the following requirements: <!-- and remove the ones that are not applicable to the current PR -->

- [ ] Tested code with current [supported SDKs](../#supported)
- [ ] Pull Request has been submitted to the documentation repository [instructions](../blob/main/Contributing.md#docs). Link: <!-- docs PR link -->
- [ ] Sample in sample app has been added / updated (for bug fixes / features)
  - [ ] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/CommunityToolkit/WindowsCommunityToolkit-design-assets)
- [ ] New major technical changes in the toolkit have or will be added to the [Wiki](https://github.com/CommunityToolkit/WindowsCommunityToolkit/wiki) e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
- [ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [ ] Header has been added to all new source files (run _build/UpdateHeaders.bat_)
- [ ] Contains **NO** breaking changes

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below.
Please note that breaking changes are likely to be rejected within minor release cycles or held until major versions. -->

## Other information

<!-- Please add any other information that might be helpful to reviewers. -->
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
controls 🎛️ feature request 📬 A request for new changes to improve functionality in progress 🚧
Projects
None yet
Development

No branches or pull requests

3 participants