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

Update VisualElement docs #16754

Merged
merged 6 commits into from
Aug 22, 2023
Merged

Update VisualElement docs #16754

merged 6 commits into from
Aug 22, 2023

Conversation

jfversluis
Copy link
Member

Description of Change

Moves the external comments to inline, adds missing comments and minor improvements.

Issues Fixed

Related to #3960

@jfversluis jfversluis added area-docs Conceptual docs, API docs, Samples t/housekeeping ♻︎ labels Aug 15, 2023
@jfversluis jfversluis requested a review from a team as a code owner August 15, 2023 11:36
@samhouts samhouts added this to the .NET 8 GA milestone Aug 15, 2023
@rmarinho rmarinho requested review from samhouts and a team August 16, 2023 13:52
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
@jknaudt21
Copy link
Contributor

We also need to document the explicit interface implementations since the docs tooling will not automatically grab these. Hopefully an <inheritdoc> should be enough.

A lot of them come from IView and IElement, and are properties we already have documented, such as Height.

To get a quick reference of the items that are missing, see: https://learn.microsoft.com/en-us/dotnet/api/microsoft.maui.controls.visualelement?view=net-maui-7.0#explicit-interface-implementations

@jfversluis jfversluis requested a review from jknaudt21 August 22, 2023 13:58
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/VisualElement/VisualElement.cs Outdated Show resolved Hide resolved
@jfversluis jfversluis requested a review from samhouts August 22, 2023 14:30
@rmarinho rmarinho enabled auto-merge (squash) August 22, 2023 17:30
@rmarinho rmarinho merged commit adfd933 into main Aug 22, 2023
@rmarinho rmarinho deleted the add-visualelement-docs branch August 22, 2023 17:40
@jfversluis jfversluis added the backport/suggested The PR author or issue review has suggested that the change should be backported. label Aug 22, 2023
@jfversluis
Copy link
Member Author

Should be relatively easy to backport and will increase the coverage of the docs in vCurrent

@hartez hartez added the backport/approved After some discussion or review, this PR or change was approved to be backported. label Aug 23, 2023
@jfversluis

This comment was marked as outdated.

1 similar comment
@jfversluis
Copy link
Member Author

/backport to net7.0

@github-actions
Copy link
Contributor

Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/5965392598

@github-actions
Copy link
Contributor

@jfversluis backporting to net7.0 failed, the patch most likely resulted in conflicts:

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

Applying: First set of comments
Using index info to reconstruct a base tree...
A	src/Controls/src/Core/VisualElement/VisualElement.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Controls/src/Core/VisualElement.cs
CONFLICT (content): Merge conflict in src/Controls/src/Core/VisualElement.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 First set of comments
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
Copy link
Contributor

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

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

@jfversluis
Copy link
Member Author

Yeah I thought as much...

hartez added a commit that referenced this pull request Aug 30, 2023
* First set of comments

* More comments

* Finish!

* PR Feedback

* More PR feedback

* Apply suggestions from code review

Co-authored-by: Samantha Houts <[email protected]>

---------

Co-authored-by: Samantha Houts <[email protected]>
jfversluis added a commit that referenced this pull request Sep 1, 2023
* Update VisualElement docs (#16754)

* First set of comments

* More comments

* Finish!

* PR Feedback

* More PR feedback

* Apply suggestions from code review

Co-authored-by: Samantha Houts <[email protected]>

---------

Co-authored-by: Samantha Houts <[email protected]>

* Fix build error

---------

Co-authored-by: Samantha Houts <[email protected]>
Co-authored-by: Gerald Versluis <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 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 backport/approved After some discussion or review, this PR or change was approved to be backported. backport/suggested The PR author or issue review has suggested that the change should be backported. fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 t/housekeeping ♻︎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants