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 Connatix Docs With Video Media Type Support #5480

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Alex404Damsa
Copy link

@Alex404Damsa Alex404Damsa commented Jul 5, 2024

🏷 Type of documentation

  • new bid adapter
  • update bid adapter
  • new feature
  • text edit only (wording, typos)
  • bugfix (code examples)
  • new examples

📋 Checklist

Copy link

netlify bot commented Jul 5, 2024

Deploy Preview for prebid-docs-preview ready!

Name Link
🔨 Latest commit 0016931
🔍 Latest deploy log https://app.netlify.com/sites/prebid-docs-preview/deploys/668be5c80f58cd0008a23fa3
😎 Deploy Preview https://deploy-preview-5480--prebid-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

dev-docs/bidders/connatix.md Show resolved Hide resolved
@@ -24,30 +24,111 @@ ortb_blocking_supported: true
sidebarType: 1
---

<div style="height: 2px; background-color: #333333; width: 100%" ></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<div style="height: 2px; background-color: #333333; width: 100%" ></div>

Those custom HTML is not needed. This makes it less consistent with the rest of the docs and thus less consumable.

Copy link
Author

Choose a reason for hiding this comment

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

Deleted the div separators.

{: .table .table-bordered .table-striped }
| Name | Scope | Description | Example | Type |
|----------------|--------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|-----------|
| sizes| required | All the sizes of the banner this ad unit can accept. | [[300, 250], [300, 600]] | Array\<Array\<integer> > |
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only a question as you seem to be an engineer. Would the typescript syntax be more readable here. So

// before
Array<Array<integer>>

// after
[number, number][]

Copy link
Author

Choose a reason for hiding this comment

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

It will indeed. I changed the type for that banner param. Thanks for pointing it out!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to hear 😊 I'll talk to the docs team if we should move to this syntax in the future.

@muuki88 muuki88 added LGTM and removed needs work labels Jul 8, 2024
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.

None yet

3 participants