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

Mistakes in docs #1354

Merged
merged 23 commits into from
Aug 9, 2017
Merged

Conversation

Vijay-Nirmal
Copy link
Contributor

@Vijay-Nirmal Vijay-Nirmal commented Jul 27, 2017

#1226

Corrected MarkdownTextBlock Example Code, Default Template and API Links
@dnfclas
Copy link

dnfclas commented Jul 27, 2017

@Vijay-Nirmal,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all .NET Foundation-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

Corrected SlidableListItem Default Template  link and Slidable spelling mistake
Corrected HttpHelper API link and changed HttpHelperRequest source code to HttpHelper source code
@dnfclas
Copy link

dnfclas commented Jul 27, 2017

@Vijay-Nirmal, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

Corrected HttpHelperResponse API Link and changed HttpHelperRequest source code to HttpHelperResponse source code
@Vijay-Nirmal
Copy link
Contributor Author

  1. There are no Default Template files for AdaptiveGridView, TextBoxMask, TextBoxRegex and WrapPanel. What should I do?
  2. What is the Device family Requirements for BindableValueHolder, DeepLinkParsers and NotificationsOverview?

@IbraheemOsama
Copy link
Member

TextBoxMask, TextBoxRegex and WrapPanel has no default template because TextBoxMask, TextBoxRegex are attached properties and WrapPanel is a Panel :)

Will have a look to get back with answers for AdaptiveGridView and the requirements for BindableValueHolder, DeepLinkParsers and NotificationsOverview

@Vijay-Nirmal
Copy link
Contributor Author

@IbraheemOsama Shall I update Menu Sample Image to a Gif? Current one is not even a Sample Image, It looks like Logo

@IbraheemOsama
Copy link
Member

@Vijay-Nirmal if you can do that would be amazing :)

@Vijay-Nirmal
Copy link
Contributor Author

Does #1297 and this PR affect each other?

@IbraheemOsama
Copy link
Member

No, it shouldn't because you're working the documentation and the other is touching code.

@dnfclas
Copy link

dnfclas commented Jul 30, 2017

@Vijay-Nirmal,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@IbraheemOsama
Copy link
Member

Clicked close by mistake :( there should be a confirmation on clicking close

@Vijay-Nirmal
Copy link
Contributor Author

Can anyone verify the below info?

Control Device Family Namespace
ExpressionBuilder Universal, 10.0.10586.0 or higher Microsoft.Toolkit.Uwp.UI.Animations
BindableValueHolder Universal, 10.0.10586.0 or higher Microsoft.Toolkit.Uwp.UI
DeepLinkParsers Universal, 10.0.10586.0 or higher Microsoft.Toolkit.Uwp
NotificationsOverview Universal, 10.0.10586.0 or higher Microsoft.Toolkit.Uwp.Notifications for C# and Microsoft.Toolkit.Uwp.Notifications.Javascript for JavaScript

@nmetulev
Copy link
Contributor

nmetulev commented Aug 6, 2017

@Vijay-Nirmal, looks correct

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Aug 7, 2017

@nmetulev

  1. There are no Default Template files for AdaptiveGridView. What should I do?
  2. Doc(RoundImageEx) has wrong Default Template Link but my Forked version has correct link

@edwinvandriel
Copy link
Contributor

@Vijay-Nirmal some extensions are moved to other namespaces in PR #1297 and doc/sample links are corrected.

@nmetulev
Copy link
Contributor

nmetulev commented Aug 7, 2017

@Vijay-Nirmal on AdaptiveGridView, I'd remove the section on default template.

@Vijay-Nirmal
Copy link
Contributor Author

@nmetulev How do I Resolve conflicts?

@Vijay-Nirmal
Copy link
Contributor Author

@edwinvandriel You didn't add the New Nuget Package(Microsoft.Toolkit.Uwp.UI.Extensions) in Nuget-Packages.md and readme.md. Shall I add it in this PR?

@nmetulev
Copy link
Contributor

nmetulev commented Aug 7, 2017

@Vijay-Nirmal, to resolve conflict, you will need to merge with dev and manually resolve the conflicts in the files.

Oh, and there is no new Nuget Package, the extensions are still under the Microsoft.Toolkit.Uwp.UI package

@Vijay-Nirmal
Copy link
Contributor Author

@nmetulev Is there a way that you can help me or it's up to me to resolve conflict?

@nmetulev
Copy link
Contributor

nmetulev commented Aug 7, 2017

I can help if you need it, can you give me write access to your fork/branch?

@nmetulev
Copy link
Contributor

nmetulev commented Aug 8, 2017

@Vijay-Nirmal, I resolved conflicts, please make sure everything looks good. Once you are ready, we can review.

@Vijay-Nirmal
Copy link
Contributor Author

Vijay-Nirmal commented Aug 8, 2017

@nmetulev Look good until I find a new mistake. Also, Thanks for resolving conflicts

@IbraheemOsama
Copy link
Member

@Vijay-Nirmal Congrats your first PR just got merged :)

@IbraheemOsama IbraheemOsama merged commit 4767277 into CommunityToolkit:dev Aug 9, 2017
@Vijay-Nirmal Vijay-Nirmal deleted the MistakesInDocs branch November 23, 2017 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants