Skip to content

Conversation

@bokelley
Copy link
Contributor

@bokelley bokelley commented Nov 3, 2025

Summary

  • Optimized vertical spacing to display more targeting keys without scrolling
  • Fixed targeting values loading error (500 error when clicking on keys)
  • Updated integration test to match new GAMInventoryDiscovery constructor

Changes

UI Optimization (style)

  • List item padding reduced from 0.75rem to 0.5rem (vertical) and 1.25rem to 0.75rem (horizontal)
  • Title font reduced to 0.9rem with tighter margin
  • Subtitle font reduced to 0.75rem with compact line height
  • Badge size reduced to 0.7rem with compact padding
  • Minimum height reduced from 50px to 40px

Bug Fix (fix)

Fixed GAMInventoryDiscovery initialization in targeting values endpoint:

  • Was incorrectly passing network_code and refresh_token directly
  • Now properly creates OAuth2 client and AdManagerClient first
  • Updated integration test test_targeting_values_endpoint.py to verify the new constructor signature
  • Fixes "Failed to load values. Please try again." error

Test plan

  • View targeting browser with many targeting keys
  • Verify more keys are visible without scrolling
  • Confirm readability is maintained
  • Check hover states still work properly
  • Click on targeting keys to load values (no longer errors)
  • Integration test passes with new constructor signature
  • Conventional commit format applied

🤖 Generated with Claude Code

bokelley and others added 2 commits November 3, 2025 09:16
Reduced padding and font sizes to fit more targeting keys on screen:
- List item padding: 0.75rem → 0.5rem vertical, 1.25rem → 0.75rem horizontal
- Title font: 0.9rem with tighter margin (0.15rem)
- Subtitle font: 0.75rem with reduced line height
- Badge size: 0.7rem with compact padding
- Min height: 50px → 40px

This allows viewing more targeting keys without scrolling.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
… endpoint

The endpoint was passing network_code and refresh_token directly to
GAMInventoryDiscovery, but the constructor expects an AdManagerClient
and tenant_id. This caused a 500 error when clicking on targeting keys.

Fixed by:
- Creating OAuth2 client with refresh token credentials
- Initializing AdManagerClient with the OAuth client
- Passing the client to GAMInventoryDiscovery constructor
- Updated integration test to match new constructor signature

This matches the pattern used in background_sync_service.py.

Fixes: "Failed to load values. Please try again." error

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley force-pushed the targeting-keys-compact branch from dc7384d to ba0fc60 Compare November 3, 2025 14:19
@bokelley bokelley merged commit 4d456b1 into main Nov 3, 2025
10 checks passed
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
* style: optimize vertical spacing in targeting keys list

Reduced padding and font sizes to fit more targeting keys on screen:
- List item padding: 0.75rem → 0.5rem vertical, 1.25rem → 0.75rem horizontal
- Title font: 0.9rem with tighter margin (0.15rem)
- Subtitle font: 0.75rem with reduced line height
- Badge size: 0.7rem with compact padding
- Min height: 50px → 40px

This allows viewing more targeting keys without scrolling.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: correct GAMInventoryDiscovery initialization in targeting values endpoint

The endpoint was passing network_code and refresh_token directly to
GAMInventoryDiscovery, but the constructor expects an AdManagerClient
and tenant_id. This caused a 500 error when clicking on targeting keys.

Fixed by:
- Creating OAuth2 client with refresh token credentials
- Initializing AdManagerClient with the OAuth client
- Passing the client to GAMInventoryDiscovery constructor
- Updated integration test to match new constructor signature

This matches the pattern used in background_sync_service.py.

Fixes: "Failed to load values. Please try again." error

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants