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

Doc: Notifications in Windows Template Studio #1238

Merged
merged 7 commits into from Oct 13, 2017

Conversation

Projects
None yet
3 participants
@mvegaca
Member

mvegaca commented Oct 5, 2017

Issue #36
Added documentation to show and explain how to extend notification feature code.

@mvegaca mvegaca referenced this pull request Oct 5, 2017

Closed

Notification Improvements #36

@mvegaca mvegaca requested a review from mrlacey Oct 5, 2017

@mvegaca

This comment has been minimized.

Show comment
Hide comment
@mvegaca

mvegaca Oct 5, 2017

Member

@mrlacey could you please make a grammar and vocabulary review of this document?

Member

mvegaca commented Oct 5, 2017

@mrlacey could you please make a grammar and vocabulary review of this document?

Tidied up some language
Still not sure the first code block is necessary though.
@mrlacey

Some improvements to the wording of the docs made.

  • some other comments
    Haven't run the sample but just looked at the code
Show outdated Hide outdated docs/getting-started-endusers.md
// the new page by passing required information in the navigation parameter
NavigationService.Navigate(_navElement, args.Arguments);
// TODO WTS: This is a sample on how to show a toast notification.

This comment has been minimized.

@mrlacey

mrlacey Oct 7, 2017

Collaborator

Should differentiate the comments added to the sample from the "TODO WTS" comments added as part of generation. The sample doesn't show things that need to be done but rather things that have been done.

@mrlacey

mrlacey Oct 7, 2017

Collaborator

Should differentiate the comments added to the sample from the "TODO WTS" comments added as part of generation. The sample doesn't show things that need to be done but rather things that have been done.

Show outdated Hide outdated ...ons/ToastNotificationSample/ToastNotificationSample/Package.appxmanifest
Show outdated Hide outdated ...Sample/ToastNotificationSample/ViewModels/ActivatedFromToastViewModel.cs
{
}
public void Initialize(ToastNotificationActivatedEventArgs args)

This comment has been minimized.

@mrlacey

mrlacey Oct 7, 2017

Collaborator

This looks important to the sample and should have some explanatory comments

@mrlacey

mrlacey Oct 7, 2017

Collaborator

This looks important to the sample and should have some explanatory comments

@mvegaca

This comment has been minimized.

Show comment
Hide comment
@mvegaca

mvegaca Oct 9, 2017

Member

@mrlacey thanks for your review. I've updated the code on this PR. Could you please merge the pull request?

Member

mvegaca commented Oct 9, 2017

@mrlacey thanks for your review. I've updated the code on this PR. Could you please merge the pull request?

@ralarcon

This comment has been minimized.

Show comment
Hide comment
@ralarcon

ralarcon Oct 10, 2017

Contributor

@mrlacey is this ready to be merged or needs other improvements? If so, please, approve the review so we can merge.

Contributor

ralarcon commented Oct 10, 2017

@mrlacey is this ready to be merged or needs other improvements? If so, please, approve the review so we can merge.

Doc reviewed and requested changes done. Needed to dismiss to progress with the PR.

@ralarcon ralarcon merged commit dae7d9d into Microsoft:dev Oct 13, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details

@mvegaca mvegaca deleted the mvegaca:Issue36-doc branch Oct 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment