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

Micorsoft Teams Sample Cookie App Js #1461

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Mohammed-MSFT
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Pawank-MSFT Pawank-MSFT left a comment

Choose a reason for hiding this comment

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

Please fix the feedbacks.

README.md Outdated
@@ -346,6 +347,7 @@ The [Teams Toolkit](https://marketplace.visualstudio.com/items?itemName=TeamsDev

[app-virtual-assistant#cs]:samples/app-virtual-assistant/csharp
[user-scope-web-application#js]:samples/user-scope-web-application/nodejs
[cookie-app#js]:samples/cookie-app/js
Copy link
Collaborator

Choose a reason for hiding this comment

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

change sample name "tab-app-cookie"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

**Page Cookie**
![shared content second user](Images/3.Page_Cookie.png)

**Seam Site Cookie**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Incorrect: Seam Site Cookie
Correct: SameSite Cookie

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

- __*This step is specific to Teams.*__
- **Edit** the `manifest.json` contained in the ./appPackage folder to replace your Microsoft App Id (that was created when you registered your app registration earlier) *everywhere* you see the place holder string `{{ANY-GUID-ID}}` (depending on the scenario the Microsoft App Id may occur multiple times in the `manifest.json`)
- **Edit** the `manifest.json` for `validDomains` and replace `{{DOMAIN-NAME}}` with base Url of your domain. E.g. if you are using ngrok it would be `https://1234.ngrok-free.app` then your DOMAIN-NAME will be `1234.ngrok-free.app` and if you are using dev tunnels then your domain will be like: `12345.devtunnels.ms`.
- **Zip** up the contents of the `appPackage` folder to create a `manifest.zip` (Make sure that zip file does not contains any subfolder otherwise you will get error while uploading your .zip package)
Copy link
Collaborator

Choose a reason for hiding this comment

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

contains should be contain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -0,0 +1,44 @@
{
"$schema": "https://developer.microsoft.com/en-us/json-schemas/teams/v1.5/MicrosoftTeams.schema.json",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Manifest version should be latest 1.19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

{
"type": "image",
"order": 100,
"url": "https://raw.githubusercontent.com/OfficeDev/Microsoft-Teams-Samples/main/samples/meetings-live-code-interview/nodejs/Images/MeetinLiveCodeInterview.gif",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Invalid sample link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

function popOutApp() {
window.open("https://teams-cookie-app.azurewebsites.net/partitioned-cookies.html");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be point local (Relative path)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Mohammed-MSFT Mohammed-MSFT added the DO NOT MERGE Do not merge the PR due to incomplete changes or feature hasn't released yet. label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Do not merge the PR due to incomplete changes or feature hasn't released yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants