Skip to content

[Docs] Added pending inline comments to Page class#20933

Merged
PureWeen merged 30 commits into
mainfrom
page-comments
Jul 18, 2024
Merged

[Docs] Added pending inline comments to Page class#20933
PureWeen merged 30 commits into
mainfrom
page-comments

Conversation

@jsuarezruiz

Copy link
Copy Markdown
Contributor

Description of Change

Added pending inline comments to Page class.

@jsuarezruiz jsuarezruiz added area-docs Conceptual docs, API docs, Samples t/housekeeping ♻︎ labels Mar 1, 2024
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner March 1, 2024 08:46
@jsuarezruiz jsuarezruiz requested review from PureWeen and rmarinho March 1, 2024 08:46

@jknaudt21 jknaudt21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have similar comments as in: #20912

Additionally, I would say to keep an eye out to not to directly mention type that a method returns in the summary if possible and instead opt to use more general language instead.

Comment thread src/Controls/src/Core/PlatformConfiguration/WindowsSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
jsuarezruiz and others added 2 commits March 5, 2024 15:39
Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>
@jsuarezruiz jsuarezruiz requested a review from jknaudt21 March 5, 2024 14:44
Comment thread src/Controls/src/Core/PlatformConfiguration/WindowsSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/WindowsSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/WindowsSpecific/Page.cs
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated

/// <include file="../../../../docs/Microsoft.Maui.Controls.PlatformConfiguration.iOSSpecific/Page.xml" path="//Member[@MemberName='PrefersHomeIndicatorAutoHidden']/Docs/*" />
/// <summary>
/// Gets a Boolean that indicates whether is allowed to hide the visual indicator for returning to the Home Screen.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar comment as SetPrefersHomeIndicatorAutoHidden


/// <include file="../../../../docs/Microsoft.Maui.Controls.PlatformConfiguration.iOSSpecific/Page.xml" path="//Member[@MemberName='SetPrefersHomeIndicatorAutoHidden'][2]/Docs/*" />
/// <summary>
/// Sets a Boolean that indicates whether is allowed to hide the visual indicator for returning to the Home Screen.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How is this property different to SetPrefersHomeIndicatorAutoHidden?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file should be entirely deleted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file should be entirely deleted

Comment thread src/Controls/src/Core/PlatformConfiguration/WindowsSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs
Comment thread src/Controls/src/Core/PlatformConfiguration/iOSSpecific/Page.cs Outdated
Comment thread src/TestUtils/src/UITest.Appium/HelperExtensions.cs Outdated
jknaudt21
jknaudt21 previously approved these changes May 7, 2024
jknaudt21
jknaudt21 previously approved these changes May 8, 2024

/// <include file="../../../../docs/Microsoft.Maui.Controls.PlatformConfiguration.WindowsSpecific/Page.xml" path="Type[@FullName='Microsoft.Maui.Controls.PlatformConfiguration.WindowsSpecific.Page']/Docs/*" />
/// <summary>
/// The page instance that Microsoft.Maui.Controls created on the Windows platform.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't really correct

This is just an extension class for setting platform specifics

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated


/// <include file="../../../../docs/Microsoft.Maui.Controls.PlatformConfiguration.WindowsSpecific/Page.xml" path="//Member[@MemberName='SetToolbarPlacement'][1]/Docs/*" />
/// <summary>
/// Sets a value that controls the placement of the toolbar.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't actually do anything right now. Is that useful to note ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

public static void MapToolbarPlacement(IToolbarHandler arg1, Toolbar arg2)

Not implemented

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How should we manage this case?

  • Include some notes in the docs?
  • Implement it?
  • Deprecate/remove it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there are 2 approaches here:

  1. We can make it invisible from the IDE by addding [EditorBrowsable(EditorBrowsableState.Never)]
  2. Adding a remark saying that this functionality hasn't been implemeted yet.

The ideal scenario would be implementing the missing functionality, but that may go beyond the scop of this PR.

@davidbritch what would be your recommended approach here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the API does nothing right now I'd hide it in the IDE.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jsuarezruiz. With this change I think we should be good to merge. I'd hate for this PR to go stale


/// <include file="../../../../docs/Microsoft.Maui.Controls.PlatformConfiguration.WindowsSpecific/Page.xml" path="//Member[@MemberName='SetToolbarPlacement'][1]/Docs/*" />
/// <summary>
/// Sets a value that controls the placement of the toolbar.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there are 2 approaches here:

  1. We can make it invisible from the IDE by addding [EditorBrowsable(EditorBrowsableState.Never)]
  2. Adding a remark saying that this functionality hasn't been implemeted yet.

The ideal scenario would be implementing the missing functionality, but that may go beyond the scop of this PR.

@davidbritch what would be your recommended approach here?

@PureWeen

Copy link
Copy Markdown
Member

/azp run

@PureWeen PureWeen enabled auto-merge (squash) July 17, 2024 14:07
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen merged commit 515c898 into main Jul 18, 2024
@PureWeen PureWeen deleted the page-comments branch July 18, 2024 20:57
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-8.0.80 and removed fixed-in-net9.0-nightly This may be available in a nightly release! labels Aug 2, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-docs Conceptual docs, API docs, Samples fixed-in-8.0.80 fixed-in-net9.0-nightly This may be available in a nightly release! t/housekeeping ♻︎

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants