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

Add IEffectControlProvider docs #20872

Merged
merged 1 commit into from
Feb 28, 2024
Merged

Add IEffectControlProvider docs #20872

merged 1 commit into from
Feb 28, 2024

Conversation

jknaudt21
Copy link
Contributor

Description of Change

Added the missing docs for the IEffectControlProvider interface


Part of documentation effort, related to #3960

@jknaudt21 jknaudt21 requested a review from a team as a code owner February 27, 2024 20:24
@jknaudt21
Copy link
Contributor Author

I was not entirely clear whether this class is meant for internal use only or open to the public. The implementation inside Element caused these doubts:

/// <summary>For internal use by .NET MAUI.</summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public IEffectControlProvider EffectControlProvider
{
get { return _effectControlProvider; }
set
{
if (_effectControlProvider == value)
return;
if (_effectControlProvider != null && _effects != null)
{
foreach (Effect effect in _effects)
effect?.SendDetached();
}
_effectControlProvider = value;
if (_effectControlProvider != null && _effects != null)
{
foreach (Effect effect in _effects)
{
if (effect != null)
AttachEffect(effect);
}
}
}
}

@jfversluis thoughts?

@jknaudt21 jknaudt21 requested a review from jfversluis February 27, 2024 20:28
@jknaudt21 jknaudt21 enabled auto-merge (squash) February 27, 2024 20:34
@jknaudt21 jknaudt21 added the area-docs Conceptual docs, API docs, Samples label Feb 28, 2024
@jfversluis
Copy link
Member

Yeah I think that's fine. The [EditorBrowsable(EditorBrowsableState.Never)] indeed means internal only although there is nothing stopping people from actually using it, it just hides it in IntelliSense. But that has historical reasons. But those are indeed internal, however they should still be documented to achieve more coverage!

@jfversluis jfversluis disabled auto-merge February 28, 2024 09:39
@jfversluis jfversluis merged commit 7875a7a into main Feb 28, 2024
40 of 47 checks passed
@jfversluis jfversluis deleted the dev/jd/ieffectsdocs branch February 28, 2024 09:39
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants