-
Notifications
You must be signed in to change notification settings - Fork 50
Create sample sites reference page #942
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
Create sample sites reference page #942
Conversation
Clicking on the code reference or the site goes to a new tab, but clicking on the docs link for the site reroutes. what are your thoughts on keeping it that way versus having the doc reference also open in a new tab? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Eiman and looks a lot better. Few things to fix. Some things (like matching titles in link destinations, no code tags in links) I didn't mark all instances.
Will need one more pass but hopefully a quick one.
One substantive query is re including the hashing tool... it changes the nature of the page. Might need input from AS on that one. Your call. LMK if you need anything.
- Code: [`uid2-examples/publisher/server_only`](https://github.com/IABTechLab/uid2-examples/tree/main/publisher/server_only) | ||
- Docs: [Publisher Integration Guide, Server-Side](https://unifiedid.com/docs/guides/integration-publisher-server-side) | ||
|
||
### Prebid.js Integrations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unnecessarily repetitive to have a major heading saying "this section includes..." and then only one smaller heading saying basically the same.
There might be more Prebid.js samples in the future no doubt, but for the moment I recommend combining this into one. Something like this:
Prebid.js Client-Side Integration
This sample is for publishers who want to generate UID2 tokens on the client side and pass them into header bidding auctions using Prebid.js. The sample integration is available at the following links:
- Site: UID2 Prebid.js Client-Side Integration Example
- Code:
uid2docs/static/examples/cstg-prebid-example
- Docs: Client-Side Integration Guide for Prebid.js
It would have to be changed back at some point perhaps -- you have the right structure -- but you're correct that it's unnecessarily repetitive when there is only one.
- [Prebid Integrations](#prebid-integrations) | ||
- [Google Ad Manager Integrations](#google-ad-manager-integrations) | ||
|
||
To explore live, working examples of UID2 implementations with source code, see [UID2 Integration Samples](../ref-info/integration-sample-sites.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add fragments of English into the Japanese files. I didn't tell you this. But just leave it. The translator will pick it up, don't worry.
If we add bits in, they could be forgotten. It's just not the process. A whole page, we have to do it because of the broken out-links problem. Code and headings we can do because they're in English anyway. But not strings of text in translated files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I should leave this as is or remove the commas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eiman-eltigani-ttd Remove line 34 which you added. So, this file will not appear in the PR: it isn't changed. It's not a necessary change to make -- the translator will add it into the Japanese files for you. Until then, the Japanese version will just be a bit behind. Sorry I wasn't clear. By "fragments" I didn't mean sentence fragments, I meant lines or paragraphs of English in a Japanese file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eiman-eltigani-ttd this one is still open.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking much better. Some inconsistencies still plus I noticed a few key things I missed before. LMK if any questions.
|
||
### Web Integrations | ||
|
||
This section summarizes the sample integrations available for publishers who want to integrate UID2 directly into their websites. For a full summary of integration options for websites, see [Web Integration Overview](https://unifiedid.com/docs/guides/integration-options-publisher-web). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Destination copy is:
Publisher Web Integration Overview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@genwhittTTD I told her to remove the word "Publisher". And the summary shouldn't have only "publisher" either. Because we shouldn't be advertising that the sample sites are only for publishers since for client-side integration, it is also available for advertisers and data providers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a larger note if you'd like to do this work at some point, it is not really correct anymore that client-side integration information is only labeled for publishers or stored under Publishers in several places since it is not only for publishers, but also advertisers and data providers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ashleysmithTTD I believe that summary is indeed for publishers... the content is organized by user. It doesn't mean that any specific implementation isn't for other audiences as well. Instead of changing the copy from the name it actually is, why not link to the Advertiser/Data Provider summary as well? https://unifiedid.com/docs/guides/integration-advertiser-dataprovider-overview. That should cover both audiences.
Happy to address any adjustments that are needed, to the docs site. And, hmm... Client-Side Integration for JavaScript is listed under Advertiser/Data Provider integrations but not listed under the implementation options http://localhost:3006/docs/guides/integration-advertiser-dataprovider-overview#summary-of-implementation-options.
I think we should get input from KK on the broader aspect of this. For now, I think the link should match the destination but no big if you feel strongly about it. I feel it's a better option, since this file is supporting multiple audiences, to link to more than one overview page, but I can live with it if you prefer not.
|
||
- Site: [UID2 Publisher Client-Side Integration Example using UID2 JavaScript SDK](https://cstg-integ.uidapi.com/) | ||
- Code: [uid2-web-integrations/examples/cstg](https://github.com/IABTechLab/uid2-web-integrations/tree/main/examples/cstg) | ||
- Docs: [Client-Side Integration Guide for JavaScript](https://unifiedid.com/docs/guides/integration-javascript-client-side) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things, on all docs links:
-
Doc, not docs, since we're linking to only one document.
-
I missed that we're including full URLs. On anything that's in the Docusaurus site, it should be a relative URl. Otherwise it opens up the destination in a new browser window... and my local build opens up the published site.
Same two edits for all docs links.
In this instance:
Docs: [Client-Side Integration Guide for JavaScript](https://unifiedid.com/docs/guides/integration-javascript-client-side)
Doc [Client-Side Integration Guide for JavaScript](../guides/integration-javascript-client-side)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this required? We wanted it to open to a destination to keep it consistent with the rest and in general makes it easier for user to read so they dont have keep losing their place in the doc. I updated it for now but lmk if this is something we can revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with Eiman here, we have the sample site and code opening in a new tab, and it makes sense for the doc to open in a new tab as well - to support the primary use case of this page in allowing users to compare multiple examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh aargh I do not agree... docs pages link to docs pages and multiple tabs can be confusing. All links to anything on the docs site are consistently relative, other than this, AFAIK. But, again, can live with this if you feel strongly about it.
|
||
This section includes a sample page for generating UID2 tokens to be passed by Prebid.js in the RTB bidstream. For details, see [UID2 Integration Overview for Prebid](../guides/integration-prebid). | ||
|
||
### Client-Side Integration with Prebid.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a section summary and individual sample header here to match the format of the other sections even though there is just one sample site
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
This tool is for developers or clients validating data preparation, such as hashing and normalization of emails before token requests. The tool is available at the following links: | ||
|
||
- Site: [UID2 Hashing Tool](https://unifiedid.com/examples/hashing-tool/) | ||
- Code: [uid2Docs/static/examples/hashing-tool](https://github.com/IABTechLab/uid2docs/tree/main/static/examples/hashing-tool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small thing here: uid2Docs/static/examples/hashing-tool > uid2docs/static/examples/hashing-tool
There is no camel case in the repo name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small extra edit. Couple of open discussions which are your call. One remove edit on integration-sample-sites.md, update the copy in the Japanese if needed, and that's it.
With those fixes we're good to go. I don't need to see it again but if you have any questions let me know. Thanks.
Summary of most recent changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thx.
No description provided.