Skip to content

fix: use brand icons in report-problem destination toggle#3018

Merged
codebanditssss merged 1 commit into
mainfrom
ui/report-problem-brand-icons
Jul 23, 2026
Merged

fix: use brand icons in report-problem destination toggle#3018
codebanditssss merged 1 commit into
mainfrom
ui/report-problem-brand-icons

Conversation

@Annieeeee11

Copy link
Copy Markdown
Collaborator

Summary

  • Swap the Report a problem destination toggle icons from generic Lucide glyphs (CircleDot / MessageSquare / Mail) to GitHub, Discord, and email brand marks
  • Keep icons inline in ReportProblemDialog (single call site) and inherit the toggle’s currentColor

Test plan

  • Open Settings → Report a problem
  • Confirm GitHub / Discord / Email show the correct logos
  • Confirm selected vs unselected states still read clearly (icon color tracks text)
  • Confirm destination switching and copy/open actions are unchanged

Before

image

After

image

Replace Lucide placeholders with GitHub, Discord, and email marks so the destination picker is recognizable at a glance.

@codebanditssss codebanditssss 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.

clean and correct. lucide dropped brand icons so inline SVGs are the right way to do this, and keeping them in the single call site is fine.

checked the render site (line 213, not in the diff): className="size-icon-sm" sizes the SVG (the new SVGs have no width/height attrs and rely on the size utility + viewBox to scale), and fill="currentColor" means the marks track the toggle's text color so selected/unselected states read correctly. each SVG hardcodes aria-hidden internally and the label comes from the adjacent {option.label} text, so no a11y regression.

one thing i verified since there's no typecheck check on this PR: passing aria-hidden to a component typed { className?: string } looked like it might be a type error, but i reproduced the exact render pattern against the real project tsc and it passes, so no latent type error.

nit (non-blocking): the call site still passes aria-hidden="true" but the components only read className and already hardcode aria-hidden, so that prop is a dead no-op you could drop.

going monochrome (currentColor) instead of full-color logos was a good call, keeps it consistent with the toggle. approving.

@codebanditssss
codebanditssss merged commit 8c46e95 into main Jul 23, 2026
4 checks passed
@Annieeeee11
Annieeeee11 deleted the ui/report-problem-brand-icons branch July 23, 2026 19:09
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