Skip to content

feat: add Anchor props to ButtonBase#526

Merged
incognitojam merged 3 commits intocommaai:masterfrom
greatgitsby:feat/btnbase-target
Apr 8, 2025
Merged

feat: add Anchor props to ButtonBase#526
incognitojam merged 3 commits intocommaai:masterfrom
greatgitsby:feat/btnbase-target

Conversation

@greatgitsby
Copy link
Copy Markdown
Contributor

@greatgitsby greatgitsby commented Apr 8, 2025

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2025

Changes:

path lines diff
./components/material/ButtonBase.tsx 10 -24

Total lines: 4505 (-24)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2025

deployed preview: https://526.connect-d5y.pages.dev

Welcome to connect! Make sure to:

  • read the contributing guidelines
  • mark your PR as a draft until it's ready to review
  • post the preview on Discord; feedback from users will speedup the PR review

Mobile

Desktop

@greatgitsby greatgitsby marked this pull request as ready for review April 8, 2025 14:16
@incognitojam
Copy link
Copy Markdown
Collaborator

incognitojam commented Apr 8, 2025

I wonder whether TypeScript would still be able to autocomplete props if we changed type ButtonBaseProps = JSX.ButtonHTMLAttributes<HTMLButtonElement> & { ... } to also include JSX.AnchorHTMLAttributes and discriminate on whether href is supplied

EDIT: seems a bit difficult since the <Show> does not narrow the types in JSX

@greatgitsby
Copy link
Copy Markdown
Contributor Author

greatgitsby commented Apr 8, 2025

i had a solution using TypeScript's typing system:

export type ButtonBaseProps<T> = T extends {
  href: string
}
  ? JSX.AnchorHTMLAttributes<HTMLAnchorElement> & {
      onClick?: (e: MouseEvent) => void
      activeClass?: string
    }
  : JSX.ButtonHTMLAttributes<HTMLButtonElement>

but yours is even more red! red diff wins!

@incognitojam
Copy link
Copy Markdown
Collaborator

Somehow this works which is pretty cool 🙂

@incognitojam incognitojam changed the title feat: add target attribute to ButtonBase feat: add Anchor props to ButtonBase Apr 8, 2025
@incognitojam incognitojam merged commit 62d533c into commaai:master Apr 8, 2025
11 of 13 checks passed
@incognitojam incognitojam added the cleanup reduce code or complexity label Apr 8, 2025
@greatgitsby greatgitsby deleted the feat/btnbase-target branch April 8, 2025 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup reduce code or complexity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants