Skip to content

[18ESP] adds simple logos#12758

Open
philcampeau wants to merge 2 commits into
tobymao:masterfrom
philcampeau:18ESP-simple-logos
Open

[18ESP] adds simple logos#12758
philcampeau wants to merge 2 commits into
tobymao:masterfrom
philcampeau:18ESP-simple-logos

Conversation

@philcampeau

Copy link
Copy Markdown
Collaborator

Before clicking "Create"

  • Branch is derived from the latest master
  • Add the pins or archive_alpha_games label if this change will break existing games
  • Code passes linter with docker compose exec rack rubocop -a
  • Tests pass cleanly with docker compose exec rack rake

Implementation Notes

Explanation of Change

Adds Simple Logos created with the generate_logos.rb script.

Screenshots

Any Assumptions / Hacks

@perwestling perwestling left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed with AI assistance by Mistral Vibe


✅ Strengths

  • Adds simple (alternative) logos for 18_esp corporations
  • Uses correct naming convention: {BASE}.alt.svg (e.g., 18_esp/A.alt.svg)
  • Files stored in correct location: public/logos/18_esp/
  • Uses generate_logos.rb script for consistency
  • Improves accessibility for players with vision difficulties
  • All other logos look fine

⚠️ Requested Changes

  • Fix text color contrast for logos with light backgrounds: MH (pink), CA (light yellow), SC (bright yellow)
  • Suggestion: Regenerate with black text (fill="#000000") for better visibility

❌ Verdict: Request changes

Please address the text color contrast issue for the three noted logos, then this will be ready for approval.

@philcampeau philcampeau requested a review from perwestling June 15, 2026 15:32
@philcampeau

Copy link
Copy Markdown
Collaborator Author

Good catch. Fixed the light-coloured tokens

@perwestling perwestling left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants