-
Notifications
You must be signed in to change notification settings - Fork 3
feat: implement LinkButton
#1105
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.
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
Conversation
🤖 Augment PR SummarySummary: Adds a new Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
darrensapalo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
|
||
| const effectiveTarget = external ? '_blank' : target | ||
| const externalRel = external ? 'noopener noreferrer' : '' | ||
| const effectiveRel = `${externalRel} ${rel}`.trim() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String interpolation includes literal "undefined" in rel attribute
Medium Severity
When rel prop is not provided (i.e., undefined), the template literal `${externalRel} ${rel}` produces the string "noopener noreferrer undefined" instead of just "noopener noreferrer". The .trim() only removes leading/trailing whitespace, not the literal "undefined" text. This affects every usage of LinkButton where rel is omitted, including the new usage in CallBooking.tsx, resulting in invalid HTML markup in the rel attribute.
Description
A link that looks like a button.
Note
Introduce
LinkButtonand consolidate button stylingLinkButtoninui/Button/LinkButton.tsx(React AriaLinkstyled as a button) withexternalprop auto-settingtarget/relBUTTON_CLASS_NAMES,ButtonSize, and newButtonStyleProps;Buttonupdated to consume shared propsAdopt
LinkButtonand API tweakCallBookingnow usesLinkButtonfor "Join call" instead of<Link><Button>useCallBookingsnow takes{ limit? }options object;BookkeepingOverviewupdated accordinglyStyles
link.scss: remove hardcoded font-size; rely on[data-size]Written by Cursor Bugbot for commit 2332d3c. This will update automatically on new commits. Configure here.
Greptile Summary
This PR introduces a new
LinkButtoncomponent that provides a link with button styling, replacing the previous pattern of wrapping aButtoninside aLink.What Changed
New Component:
LinkButtoncomponent insrc/components/ui/Button/LinkButton.tsxthat combines link functionality with button stylingLinkcomponent for accessibility while applying button visual stylesexternalprop for convenient handling of external links (automatically setstarget="_blank"andrel="noopener noreferrer")Button Component Updates:
BUTTON_CLASS_NAMES,ButtonSize, and created a newButtonStylePropstype to enable code reuse inLinkButtonButtonStylePropstypeLink Styles Cleanup:
font-size: var(--text-md)from link.scss to allow proper size control via data attributesImplementation:
CallBookingcomponent to use the newLinkButtoninstead of<Link><Button>pattern for the "Join call" actionuseCallBookingshook API to use an options object parameter instead of positional parameters for better extensibilityArchitecture Benefits
The new
LinkButtoncomponent provides better semantics and developer experience:<Link><Button>nestingAreas for Improvement
While the implementation is functionally correct, there are opportunities to strengthen this addition:
user-select: nonewhich may impact link UX (see review comments)