Skip to content

fix(ui): improve design of buttons/links with icons #1542

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.

Sign up for GitHub

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

Open
NeeshSamsi wants to merge 7 commits into
base: main
Choose a base branch
from
+43 −25
Open

fix(ui): improve design of buttons/links with icons #1542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ccebf91
fix: improve design of buttons/links with icons (#1255)
NeeshSamsi Feb 18, 2026
bb0cc7e
feat(ui): auto-detect icon-only buttons for consistent spacing
NeeshSamsi Feb 18, 2026
39b1337
fix(ui): use aria-label instead of sr-only spans for icon-only buttons
NeeshSamsi Feb 18, 2026
ff4be8b
fix(ui): make homepage search button square on mobile
NeeshSamsi Feb 18, 2026
28016fc
fix: Apply suggestion from @carwack for better TS support
NeeshSamsi Feb 27, 2026
66a9a1d
Merge branch 'main' into fix/icon-button-padding
NeeshSamsi Feb 27, 2026
4d3d6cd
[autofix.ci] apply automated fixes
autofix-ci[bot] Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions app/components/Button/Base.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ const props = withDefaults(
)

const el = useTemplateRef('el')
const slots = defineSlots<{
default?: () => unknown
}>()
const iconOnly = computed(() => !!props.classicon && !slots.default)
Comment on lines +28 to +31
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n app/components/Button/Base.vue

Repository: npmx-dev/npmx.dev

Length of output: 3513

🏁 Script executed:

# Search for ButtonBase usages with classicon
rg -n 'classicon' app/components --type vue -B 2 -A 10

Repository: npmx-dev/npmx.dev

Length of output: 88

🏁 Script executed:

# Search for icon-only components with sr-only or responsive hidden labels
rg -n 'sr-only|max-sm:sr-only|sm:not-sr-only' app/components --type vue -B 3 -A 3

Repository: npmx-dev/npmx.dev

Length of output: 88

🏁 Script executed:

# Look for Button/LinkBase usages in pages and other components
rg -n '<(Button|Link)Base' app --type vue | head -20

Repository: npmx-dev/npmx.dev

Length of output: 88

🏁 Script executed:

# Search for ButtonBase and LinkBase usages with more context
rg -n 'ButtonBase|LinkBase' app/components app/pages --glob '*.vue' -A 5 | head -60

Repository: npmx-dev/npmx.dev

Length of output: 3848

🏁 Script executed:

# Search for sr-only and responsive visibility patterns
rg -n 'sr-only|max-sm:sr-only|sm:not-sr-only' app/components app/pages --glob '*.vue' -B 2 -A 2

Repository: npmx-dev/npmx.dev

Length of output: 20276

🏁 Script executed:

# Look specifically for patterns combining classicon with hidden content
rg -n 'classicon' app/components app/pages --glob '*.vue' -A 15 | grep -A 15 'sr-only\|max-sm'

Repository: npmx-dev/npmx.dev

Length of output: 6555

Icon-only detection doesn't account for responsive hidden labels.

At line 31, iconOnly is inferred solely from slot existence. Components with label slots hidden at certain breakpoints (e.g., sr-only sm:not-sr-only) are treated as non-icon-only, causing inconsistent padding. Call-sites with responsive labels require ad-hoc spacing overrides, as seen in app/pages/index.vue:95 with max-sm:p-2.

Consider adding an optional iconOnly prop to permit explicit override whilst preserving automatic detection:

Proposed approach 
 const props = withDefaults(
   defineProps<{
@@
     classicon?: IconClass
+    /** Override icon-only spacing when label content is visually hidden via CSS. */
+    iconOnly?: boolean
   }>(),
@@
 const slots = defineSlots<{
   default?: () => unknown
 }>()
-const iconOnly = computed(() => !!props.classicon && !slots.default)
+const inferred = computed(() => !!props.classicon && !slots.default)
+const iconOnly = computed(() => props.iconOnly ?? inferred.value)

Responsive hidden labels exist in multiple call-sites (e.g., app/pages/index.vue:98, app/pages/package/[[org]]/[name].vue:859) and should benefit from explicit control.


const keyboardShortcutsEnabled = useKeyboardShortcuts()

Expand All @@ -41,8 +45,14 @@ defineExpose({
:class="{
'inline-flex': !block,
'flex': block,
'text-sm px-4 py-2': size === 'medium',
'text-xs px-2 py-0.5': size === 'small',
'text-sm py-2': size === 'medium' && !iconOnly,
'text-sm p-2': size === 'medium' && !!iconOnly,
'px-4': size === 'medium' && !classicon && !iconOnly,
'ps-3 pe-4': size === 'medium' && !!classicon && !iconOnly,
'text-xs py-0.5': size === 'small' && !iconOnly,
'text-xs p-0.5': size === 'small' && !!iconOnly,
'px-2': size === 'small' && !classicon && !iconOnly,
'ps-1.5 pe-2': size === 'small' && !!classicon && !iconOnly,
'bg-transparent text-fg hover:enabled:(bg-fg/10) focus-visible:enabled:(bg-fg/10) aria-pressed:(bg-fg/10 border-fg/20 hover:enabled:(bg-fg/20 text-fg/50))':
variant === 'secondary',
'text-bg bg-fg hover:enabled:(bg-fg/50) focus-visible:enabled:(bg-fg/50) aria-pressed:(bg-fg text-bg border-fg hover:enabled:(text-bg/50))':
Expand Down
22 changes: 18 additions & 4 deletions app/components/Link/Base.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const isLink = computed(() => props.variant === 'link')
const isButton = computed(() => !isLink.value)
const isButtonSmall = computed(() => props.size === 'small' && !isLink.value)
const isButtonMedium = computed(() => props.size === 'medium' && !isLink.value)
const slots = useSlots()
const iconOnly = computed(() => !!props.classicon && !slots.default)
const keyboardShortcutsEnabled = useKeyboardShortcuts()
</script>

Expand All @@ -70,8 +72,14 @@ const keyboardShortcutsEnabled = useKeyboardShortcuts()
'inline-flex': !block,
'opacity-50 gap-x-1 items-center justify-center font-mono border border-transparent rounded-md':
isButton,
'text-sm px-4 py-2': isButtonMedium,
'text-xs px-2 py-0.5': isButtonSmall,
'text-sm py-2': isButtonMedium && !iconOnly,
'text-sm p-2': isButtonMedium && !!iconOnly,
'px-4': isButtonMedium && !classicon && !iconOnly,
'ps-3 pe-4': isButtonMedium && !!classicon && !iconOnly,
'text-xs py-0.5': isButtonSmall && !iconOnly,
'text-xs p-0.5': isButtonSmall && !!iconOnly,
'px-2': isButtonSmall && !classicon && !iconOnly,
'ps-1.5 pe-2': isButtonSmall && !!classicon && !iconOnly,
'text-bg bg-fg': variant === 'button-primary',
'bg-transparent text-fg': variant === 'button-secondary',
}"
Expand All @@ -90,8 +98,14 @@ const keyboardShortcutsEnabled = useKeyboardShortcuts()
isLink,
'justify-center font-mono border border-border rounded-md transition-all duration-200':
isButton,
'text-sm px-4 py-2': isButtonMedium,
'text-xs px-2 py-0.5': isButtonSmall,
'text-sm py-2': isButtonMedium && !iconOnly,
'text-sm p-2': isButtonMedium && iconOnly,
'px-4': isButtonMedium && !classicon && !iconOnly,
'ps-3 pe-4': isButtonMedium && !!classicon && !iconOnly,
'text-xs py-0.5': isButtonSmall && !iconOnly,
'text-xs p-0.5': isButtonSmall && iconOnly,
'px-2': isButtonSmall && !classicon && !iconOnly,
'ps-1.5 pe-2': isButtonSmall && !!classicon && !iconOnly,
'bg-transparent text-fg hover:(bg-fg/10 text-accent) focus-visible:(bg-fg/10 text-accent) aria-[current=true]:(bg-fg/10 text-accent border-fg/20 hover:enabled:(bg-fg/20 text-fg/50))':
variant === 'button-secondary',
'text-bg bg-fg hover:(bg-fg/50 text-accent) focus-visible:(bg-fg/50) aria-current:(bg-fg text-bg border-fg hover:enabled:(text-bg/50))':
Expand Down
10 changes: 4 additions & 6 deletions app/components/Package/Dependencies.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,20 +150,18 @@ const numberFormatter = useNumberFormatter()
:to="packageRoute(dep, getVulnerableDepInfo(dep)!.version)"
class="shrink-0"
:class="SEVERITY_TEXT_COLORS[getHighestSeverity(getVulnerableDepInfo(dep)!.counts)]"
:aria-label="$t('package.dependencies.view_vulnerabilities')"
:title="`${getVulnerableDepInfo(dep)!.counts.total} vulnerabilities`"
classicon="i-lucide:shield-check"
>
<span class="sr-only">{{ $t('package.dependencies.view_vulnerabilities') }}</span>
</LinkBase>
/>
<LinkBase
v-if="getDeprecatedDepInfo(dep)"
:to="packageRoute(dep, getDeprecatedDepInfo(dep)!.version)"
class="shrink-0 text-purple-700 dark:text-purple-500"
:aria-label="$t('package.deprecated.label')"
:title="getDeprecatedDepInfo(dep)!.message"
classicon="i-lucide:octagon-alert"
>
<span class="sr-only">{{ $t('package.deprecated.label') }}</span>
</LinkBase>
/>
<LinkBase
:to="packageRoute(dep, version)"
class="block truncate"
Expand Down
5 changes: 2 additions & 3 deletions app/components/Package/Versions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,12 +501,11 @@ function majorGroupContainsCurrent(group: (typeof otherMajorGroups.value)[0]): b
<ButtonBase
variant="secondary"
class="text-fg-subtle hover:text-fg transition-colors min-w-6 min-h-6 -m-1 p-1 rounded"
:aria-label="$t('package.downloads.community_distribution')"
:title="$t('package.downloads.community_distribution')"
classicon="i-lucide:file-stack"
@click="openDistributionModal"
>
<span class="sr-only">{{ $t('package.downloads.community_distribution') }}</span>
</ButtonBase>
/>
</template>
<div class="space-y-0.5 min-w-0">
<!-- Semver range filter -->
Expand Down
5 changes: 2 additions & 3 deletions app/components/Package/WeeklyDownloadStats.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,10 @@ const config = computed<VueUiSparklineConfig>(() => {
type="button"
@click="openChartModal"
class="text-fg-subtle hover:text-fg transition-colors duration-200 inline-flex items-center justify-center min-w-6 min-h-6 -m-1 p-1 focus-visible:outline-accent/70 rounded"
:aria-label="$t('package.trends.title')"
:title="$t('package.trends.title')"
classicon="i-lucide:chart-line"
>
<span class="sr-only">{{ $t('package.trends.title') }}</span>
</ButtonBase>
/>
<span v-else-if="isLoadingWeeklyDownloads" class="min-w-6 min-h-6 -m-1 p-1" />
</template>

Expand Down
2 changes: 1 addition & 1 deletion app/pages/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ defineOgImageComponent('Default', {
<ButtonBase
type="submit"
variant="primary"
class="absolute inset-ie-2 border-transparent"
class="absolute inset-ie-2 border-transparent max-sm:p-2"
classicon="i-lucide:search"
>
<span class="sr-only sm:not-sr-only">
Expand Down
10 changes: 4 additions & 6 deletions app/pages/package/[[org]]/[name].vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,21 +1123,19 @@ const showSkeleton = shallowRef(false)
variant="button-secondary"
size="small"
:to="`https://npmgraph.js.org/?q=${pkg.name}`"
:aria-label="$t('package.stats.view_dependency_graph')"
:title="$t('package.stats.view_dependency_graph')"
classicon="i-lucide:network -rotate-90"
>
<span class="sr-only">{{ $t('package.stats.view_dependency_graph') }}</span>
</LinkBase>
/>

<LinkBase
variant="button-secondary"
size="small"
:to="`https://node-modules.dev/grid/depth#install=${pkg.name}${resolvedVersion ? `@${resolvedVersion}` : ''}`"
:aria-label="$t('package.stats.inspect_dependency_tree')"
:title="$t('package.stats.inspect_dependency_tree')"
classicon="i-lucide:table"
>
<span class="sr-only">{{ $t('package.stats.inspect_dependency_tree') }}</span>
</LinkBase>
/>
</ButtonGroup>
</dd>
</div>
Expand Down
Loading