Skip to content

[codex] Show ad title when ad URL is missing #546

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.

Sign up for GitHub

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

Merged
jahooma merged 2 commits into from
Apr 25, 2026
+54 −4
Merged

[codex] Show ad title when ad URL is missing #546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4936b9b
Show ad title without URL
jahooma Apr 25, 2026
4444018
Update cli/src/components/choice-ad-banner.tsx
jahooma Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions cli/src/components/__tests__/choice-ad-banner.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, expect, test } from 'bun:test'

import { getAdDisplayLabel } from '../choice-ad-banner'

describe('choice ad banner display label', () => {
test('uses the display domain when the ad has a URL', () => {
expect(
getAdDisplayLabel({
title: 'Example Sponsor',
url: 'https://www.example.com/path',
}),
).toEqual({ text: 'example.com', variant: 'domain' })
})

test('uses the ad title when the ad has no URL', () => {
expect(
getAdDisplayLabel({
title: 'Example Sponsor',
url: '',
}),
).toEqual({ text: 'Example Sponsor', variant: 'title' })
})
})
35 changes: 31 additions & 4 deletions cli/src/components/choice-ad-banner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ function truncateToLines(text: string, lineWidth: number, maxLines: number): str
return text.slice(0, maxChars - 1) + '…'
}

const extractDomain = (url: string): string => {
function truncateToWidth(text: string, width: number): string {
if (width <= 0) return ''
if (text.length <= width) return text
return text.slice(0, width - 1) + '…'
}
Comment thread
jahooma marked this conversation as resolved.

export const extractDomain = (url: string): string => {
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 25, 2026

Choose a reason for hiding this comment

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

P2 Unnecessary export on extractDomain

extractDomain is not imported by the test file (only getAdDisplayLabel is), so the export added here is unneeded. Keeping it private prevents unintended coupling to callers outside this module.

Suggested change
export const extractDomain = (url: string): string => {
const extractDomain = (url: string): string => {
Prompt To Fix With AI 
This is a comment left during a code review.
Path: cli/src/components/choice-ad-banner.tsx
Line: 35

Comment:
**Unnecessary export on `extractDomain`**

`extractDomain` is not imported by the test file (only `getAdDisplayLabel` is), so the `export` added here is unneeded. Keeping it private prevents unintended coupling to callers outside this module.

```suggestion
const extractDomain = (url: string): string => {
```

How can I resolve this? If you propose a fix, please make it concise.

try {
const parsed = new URL(url)
return parsed.hostname.replace(/^www\./, '')
Expand All @@ -34,6 +40,17 @@ const extractDomain = (url: string): string => {
}
}

export function getAdDisplayLabel(
ad: Pick<AdResponse, 'title' | 'url'>,
): { text: string; variant: 'domain' | 'title' } {
const url = ad.url.trim()
if (url) {
return { text: extractDomain(url), variant: 'domain' }
}

return { text: ad.title.trim() || 'Sponsored', variant: 'title' }
}

/**
* Calculate evenly distributed column widths that sum exactly to availableWidth.
* Distributes remainder pixels across the first N columns so there's no gap.
Expand Down Expand Up @@ -89,8 +106,10 @@ export const ChoiceAdBanner: React.FC<ChoiceAdBannerProps> = ({ ads, onImpressio
>
{visibleAds.map((ad, i) => {
const isHovered = hoveredIndex === i
const domain = extractDomain(ad.url)
const ctaText = ad.cta || ad.title || 'Learn more'
const label = getAdDisplayLabel(ad)
const labelMaxWidth = Math.max(0, widths[i] - ctaText.length - 5)
const labelText = truncateToWidth(label.text, labelMaxWidth)

return (
<Button
Expand Down Expand Up @@ -130,8 +149,16 @@ export const ChoiceAdBanner: React.FC<ChoiceAdBannerProps> = ({ ads, onImpressio
>
{` ${ctaText} `}
</text>
<text style={{ fg: theme.muted, attributes: TextAttributes.UNDERLINE }}>
{domain}
<text
style={{
fg: theme.muted,
attributes:
label.variant === 'domain'
? TextAttributes.UNDERLINE
: TextAttributes.DIM,
}}
>
{labelText}
</text>

</box>
Expand Down
Loading