Add ViewHandler & IViewHandler docs #17167

jfversluis · 2023-09-01T12:32:43Z

Description of Change

Adds inline API docs

Issues Fixed

Related to #3960

jfversluis · 2023-09-01T12:33:26Z

src/Core/src/Handlers/IViewHandler.cs

 	public interface IViewHandler : IElementHandler
 	{
+		/// <summary>
+		/// Gets or sets whether the <see cref="IElementHandler.PlatformView"/> has a view that it is contained in.
+		/// </summary>
 		bool HasContainer { get; set; }


Should we add a remark on when this is applicable? What should the consuming developer know about this?

What controls don't have a container? If the list is small we can just add a small remark saying something like "X, Y, and Z will never have a container"

jfversluis · 2023-09-01T12:34:30Z

src/Core/src/Handlers/View/ViewHandler.cs

 	public abstract partial class ViewHandler : ElementHandler, IViewHandler
 	{
+		/// <summary>
+		/// A dictionary that maps the virtual view properties to their platform view counterparts.
+		/// </summary>


I feel we can add a little more explanation for this, especially since it's one of the key concepts of the architecture, but no inspiration currently.

I second this idea. Not very familiar with the property mapper so we might need someone more knowledgeable to help us out 😅

src/Core/src/Handlers/IViewHandler.cs

jknaudt21 · 2023-09-01T19:00:51Z

src/Core/src/Handlers/IViewHandler.cs

 		object? ContainerView { get; }

+		/// <summary>
+		/// Gets the virtual view (.NET MAUI layer) that is managed by this handler.


I think its ok to not include "(.NET MAUI layer)" since a fair amount of the existing docs already just say "virtual view .... ".

Maybe we can add it here as a remark though, since this is the base class for most handlers.

This comment pertains to similar places in this PR too. At the same time, I do like that it specifies that this is the MAUI layer and wish that the other existing comments were like that 😵. I'll leave this up to you!

jknaudt21 · 2023-09-01T19:01:59Z

src/Core/src/Handlers/View/ViewHandler.cs

 	public abstract partial class ViewHandler : ElementHandler, IViewHandler
 	{
+		/// <summary>
+		/// A dictionary that maps the virtual view properties to their platform view counterparts.
+		/// </summary>


I second this idea. Not very familiar with the property mapper so we might need someone more knowledgeable to help us out 😅

src/Core/src/Handlers/View/ViewHandler.cs

src/Core/src/Handlers/IViewHandler.cs

jknaudt21 · 2023-09-01T19:10:29Z

src/Core/src/Handlers/IViewHandler.cs

 		new IView? VirtualView { get; }

+		/// <summary>
+		/// Computes the actual size of a view based on the desired size and constraints. 


Maybe specify that this is the on-screen size? Perhaps as a remark.

jsuarezruiz · 2023-09-11T16:44:19Z

/azp run

azure-pipelines · 2023-09-11T16:44:34Z

Azure Pipelines successfully started running 2 pipeline(s).

ghost · 2023-09-11T23:22:35Z

Hi @jfversluis. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time.

Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>

jknaudt21

I still think there are a couple of items that need to be addressed on this PR, but nothing major enough to prolong the merge.

PureWeen · 2023-10-24T19:51:09Z

/backport to net8.0

github-actions · 2023-10-24T19:51:23Z

Started backporting to net8.0: https://github.com/dotnet/maui/actions/runs/6631838768

PureWeen · 2023-10-24T19:51:25Z

/backport to release/8.0.1xx-rc2.2

github-actions · 2023-10-24T19:51:41Z

Started backporting to release/8.0.1xx-rc2.2: https://github.com/dotnet/maui/actions/runs/6631841147

github-actions · 2023-10-24T19:52:02Z

@PureWeen backporting to net8.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add ViewHandler & IViewHandler inline docs
.git/rebase-apply/patch:35: trailing whitespace.
		/// Computes the actual size of a view based on the desired size and constraints. 
.git/rebase-apply/patch:117: trailing whitespace.
		/// 
.git/rebase-apply/patch:123: trailing whitespace.
		/// 
.git/rebase-apply/patch:129: trailing whitespace.
		/// Gets the 
warning: 4 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	src/Core/src/Handlers/IViewHandler.cs
M	src/Core/src/Handlers/View/ViewHandler.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Core/src/Handlers/View/ViewHandler.cs
CONFLICT (content): Merge conflict in src/Core/src/Handlers/View/ViewHandler.cs
Auto-merging src/Core/src/Handlers/IViewHandler.cs
CONFLICT (content): Merge conflict in src/Core/src/Handlers/IViewHandler.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add ViewHandler & IViewHandler inline docs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-10-24T19:52:03Z

@PureWeen an error occurred while backporting to net8.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

github-actions · 2023-10-24T19:52:03Z

@PureWeen backporting to release/8.0.1xx-rc2.2 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add ViewHandler & IViewHandler inline docs
.git/rebase-apply/patch:35: trailing whitespace.
		/// Computes the actual size of a view based on the desired size and constraints. 
.git/rebase-apply/patch:117: trailing whitespace.
		/// 
.git/rebase-apply/patch:123: trailing whitespace.
		/// 
.git/rebase-apply/patch:129: trailing whitespace.
		/// Gets the 
warning: 4 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	src/Core/src/Handlers/IViewHandler.cs
M	src/Core/src/Handlers/View/ViewHandler.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Core/src/Handlers/View/ViewHandler.cs
CONFLICT (content): Merge conflict in src/Core/src/Handlers/View/ViewHandler.cs
Auto-merging src/Core/src/Handlers/IViewHandler.cs
CONFLICT (content): Merge conflict in src/Core/src/Handlers/IViewHandler.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add ViewHandler & IViewHandler inline docs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-10-24T19:52:04Z

@PureWeen an error occurred while backporting to release/8.0.1xx-rc2.2, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

jfversluis added area-docs Conceptual docs, API docs, Samples t/housekeeping ♻︎ labels Sep 1, 2023

jfversluis added this to the .NET 8 GA milestone Sep 1, 2023

jfversluis requested a review from jknaudt21 September 1, 2023 12:32

jfversluis requested a review from a team as a code owner September 1, 2023 12:32

jfversluis requested review from StephaneDelcroix and rachelkang September 1, 2023 12:32

jfversluis commented Sep 1, 2023

View reviewed changes

jknaudt21 requested changes Sep 1, 2023

View reviewed changes

samhouts added the s/pr-needs-author-input PR needs an update from the author label Sep 11, 2023

StephaneDelcroix approved these changes Sep 14, 2023

View reviewed changes

samhouts modified the milestones: .NET 8 GA, Under Consideration Sep 19, 2023

jfversluis and others added 3 commits September 27, 2023 15:33

Add ViewHandler & IViewHandler inline docs

c3046f9

Update ViewHandler.cs

fe38890

Apply suggestions from code review

780d9d8

Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>

jfversluis force-pushed the add-viewhandler-docs branch from 516cf44 to 780d9d8 Compare September 27, 2023 13:35

jknaudt21 approved these changes Sep 27, 2023

View reviewed changes

jfversluis merged commit 6326bf7 into main Sep 28, 2023
47 checks passed

jfversluis deleted the add-viewhandler-docs branch September 28, 2023 13:01

github-actions bot locked and limited conversation to collaborators Dec 6, 2023

samhouts removed this from the Under Consideration milestone Jul 1, 2024

samhouts added the fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511 label Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ViewHandler & IViewHandler docs #17167

Add ViewHandler & IViewHandler docs #17167

jfversluis commented Sep 1, 2023

jfversluis Sep 1, 2023

jknaudt21 Sep 5, 2023

jfversluis Sep 1, 2023

jknaudt21 Sep 1, 2023

jknaudt21 Sep 1, 2023

jknaudt21 Sep 1, 2023

jknaudt21 Sep 1, 2023

jsuarezruiz commented Sep 11, 2023

azure-pipelines bot commented Sep 11, 2023

ghost commented Sep 11, 2023

jknaudt21 left a comment

PureWeen commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

PureWeen commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

Add ViewHandler & IViewHandler docs #17167

Add ViewHandler & IViewHandler docs #17167

Conversation

jfversluis commented Sep 1, 2023

Description of Change

Issues Fixed

jfversluis Sep 1, 2023

Choose a reason for hiding this comment

jknaudt21 Sep 5, 2023

Choose a reason for hiding this comment

jfversluis Sep 1, 2023

Choose a reason for hiding this comment

jknaudt21 Sep 1, 2023

Choose a reason for hiding this comment

jknaudt21 Sep 1, 2023

Choose a reason for hiding this comment

jknaudt21 Sep 1, 2023

Choose a reason for hiding this comment

jknaudt21 Sep 1, 2023

Choose a reason for hiding this comment

jsuarezruiz commented Sep 11, 2023

azure-pipelines bot commented Sep 11, 2023

ghost commented Sep 11, 2023

jknaudt21 left a comment

Choose a reason for hiding this comment

PureWeen commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

PureWeen commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023

github-actions bot commented Oct 24, 2023