fix: change responsive button breakpoints and use them where necessary#2093
Merged
im-adithya merged 2 commits intomasterfrom Feb 26, 2026
Merged
fix: change responsive button breakpoints and use them where necessary#2093im-adithya merged 2 commits intomasterfrom
im-adithya merged 2 commits intomasterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThe changes adjust responsive design breakpoints from large-screen (lg) to medium-screen (md) thresholds in three button components and replace standard button implementations with their responsive counterparts in three screen components, improving layout behavior on smaller displays. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
im-adithya
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/ResponsiveLinkButton.tsx (1)
31-38:⚠️ Potential issue | 🟠 MajorAdd an accessible name to the icon-only link button.
The mobile variant is icon-only (
md:hidden), so it should explicitly exposearia-label.♿ Suggested fix
<LinkButton {...props} size="icon" className="md:hidden" + aria-label={text} variant={variant} > - <Icon /> + <Icon aria-hidden="true" /> </LinkButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ResponsiveLinkButton.tsx` around lines 31 - 38, The icon-only mobile variant in ResponsiveLinkButton lacks an accessible name; update the LinkButton render in ResponsiveLinkButton to pass an explicit aria-label (or ariaLabel prop accepted by LinkButton) when it renders the icon-only version so screen readers can announce the button—ensure you use a meaningful label (e.g., from props.label, props['aria-label'], or a dedicated prop) and keep passing {...props} along with size="icon", className="md:hidden", and variant={variant} so the accessible name is always present for the Icon-only button.frontend/src/components/ResponsiveExternalLinkButton.tsx (1)
31-38:⚠️ Potential issue | 🟠 MajorIcon-only external-link variant should include an accessible label.
The
md:hiddenbutton renders only an icon; addaria-labelso screen readers can identify the action.♿ Suggested fix
<ExternalLinkButton {...props} size="icon" className="md:hidden" + aria-label={text} variant={variant} > - <Icon /> + <Icon aria-hidden="true" /> </ExternalLinkButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ResponsiveExternalLinkButton.tsx` around lines 31 - 38, The icon-only ExternalLinkButton rendered in ResponsiveExternalLinkButton lacks an accessible label; update the icon-only branch (the ExternalLinkButton with size="icon" and className="md:hidden") to include an aria-label prop. Pass-through any incoming aria-label (props["aria-label"] or props.ariaLabel) if provided, otherwise supply a sensible default like "Open external link" so screen readers can identify the action; ensure you add the aria-label on the ExternalLinkButton that renders <Icon />.frontend/src/components/ResponsiveButton.tsx (1)
24-44:⚠️ Potential issue | 🟠 MajorAdd a fallback accessible name for mobile icon-only state.
When the label is hidden below
md, this becomes icon-only and may be unnamed for assistive tech. Set a defaultaria-labelfromtext.♿ Suggested fix
const ResponsiveButton = ({ icon: Icon, text, variant, asChild, children, className, ...props }: Props) => { const content = ( <> - <Icon /> + <Icon aria-hidden="true" /> <span className="hidden md:inline">{text}</span> </> ); return ( <Button {...props} + aria-label={props["aria-label"] ?? text} variant={variant} asChild={asChild} className={cn( className, "max-md:size-9" /* apply size="icon" only for mobile */ )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ResponsiveButton.tsx` around lines 24 - 44, The icon-only mobile state can be inaccessible because the visible label is hidden; ensure an accessible name is provided by passing a default aria-label derived from text when none was supplied: add logic in ResponsiveButton (around the Button JSX and the React.cloneElement branch) to compute const ariaLabel = props["aria-label"] ?? text and pass ariaLabel to the Button via aria-label={ariaLabel}, and when asChild is true also merge aria-label into the cloned child (only if the child lacks one) so the rendered element always has an accessible name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/components/ResponsiveButton.tsx`:
- Around line 24-44: The icon-only mobile state can be inaccessible because the
visible label is hidden; ensure an accessible name is provided by passing a
default aria-label derived from text when none was supplied: add logic in
ResponsiveButton (around the Button JSX and the React.cloneElement branch) to
compute const ariaLabel = props["aria-label"] ?? text and pass ariaLabel to the
Button via aria-label={ariaLabel}, and when asChild is true also merge
aria-label into the cloned child (only if the child lacks one) so the rendered
element always has an accessible name.
In `@frontend/src/components/ResponsiveExternalLinkButton.tsx`:
- Around line 31-38: The icon-only ExternalLinkButton rendered in
ResponsiveExternalLinkButton lacks an accessible label; update the icon-only
branch (the ExternalLinkButton with size="icon" and className="md:hidden") to
include an aria-label prop. Pass-through any incoming aria-label
(props["aria-label"] or props.ariaLabel) if provided, otherwise supply a
sensible default like "Open external link" so screen readers can identify the
action; ensure you add the aria-label on the ExternalLinkButton that renders
<Icon />.
In `@frontend/src/components/ResponsiveLinkButton.tsx`:
- Around line 31-38: The icon-only mobile variant in ResponsiveLinkButton lacks
an accessible name; update the LinkButton render in ResponsiveLinkButton to pass
an explicit aria-label (or ariaLabel prop accepted by LinkButton) when it
renders the icon-only version so screen readers can announce the button—ensure
you use a meaningful label (e.g., from props.label, props['aria-label'], or a
dedicated prop) and keep passing {...props} along with size="icon",
className="md:hidden", and variant={variant} so the accessible name is always
present for the Icon-only button.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/components/ResponsiveButton.tsxfrontend/src/components/ResponsiveExternalLinkButton.tsxfrontend/src/components/ResponsiveLinkButton.tsxfrontend/src/screens/internal-apps/ZapPlanner.tsxfrontend/src/screens/onchain/DepositBitcoin.tsxfrontend/src/screens/peers/Peers.tsx
Member
Author
|
I'll merge this as there aren't any comments apart from aria labels, only thing worth mentioning is the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1816
The responsive buttons look good till
mdwidth, so changed the breakpoint fromlgtomd(verified ✅ on all screens)Replaced all places where
AppHeaderuses non responsive buttonsSummary by CodeRabbit