Skip to content
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

feat: icon only buttons #4642

Merged
merged 13 commits into from
Jan 30, 2025
Merged

feat: icon only buttons #4642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
408f31e
feat: icon only buttons
connoratrug Jan 27, 2025
a8e8f1f
Merge branch 'master' into feat/icon-only-buttons
connoratrug Jan 29, 2025
d5df284
Merge branch 'master' into feat/icon-only-buttons
connoratrug Jan 29, 2025
8b17f65
Merge branch 'master' into feat/icon-only-buttons
connoratrug Jan 29, 2025
d4ed6ac
Merge branch 'master' into feat/icon-only-buttons
connoratrug Jan 29, 2025
ecf502b
remove unneeded span
connoratrug Jan 29, 2025
90f6efe
require label for icon only btn
connoratrug Jan 29, 2025
66bea4c
fix directory e2e tests
connoratrug Jan 29, 2025
d7d45d5
update config
connoratrug Jan 29, 2025
c95bc82
one more time with feeling
connoratrug Jan 29, 2025
f35ff17
Merge branch 'master' into feat/icon-only-buttons
connoratrug Jan 29, 2025
c92fb22
Update config.yml
connoratrug Jan 29, 2025
601d0c2
Update getAppRoute.ts
connoratrug Jan 29, 2025
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
23 changes: 18 additions & 5 deletions apps/tailwind-components/components/Button.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,25 @@ const props = withDefaults(
icon?: string;
iconPosition?: ButtonIconPosition;
disabled?: boolean;
iconOnly?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

this is not possible automatic by checken if there are child elements?

Copy link
Member

Choose a reason for hiding this comment

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

otherwise I don't mind for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i tried something like that , but it did not work , i'll have a look if de links contain additional info , i'm not even really sure i like the slot option in the case as it can get a bit weird when passing non string stuff

Copy link
Contributor

Choose a reason for hiding this comment

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

You might be able to use UseTemplateRef and evaluate if there is inner html.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is also possible to use a named slot and check if that has content.

tooltip?: string;
}>(),
{
type: "primary",
size: "medium",
label: "",
iconPosition: "left",
disabled: false,
iconOnly: false,
}
);

watchEffect(() => {
if (props.iconOnly && (props.label === "" || props.label === undefined)) {
console.error("Icon only buttons must have a label");
}
});

const COLOR_MAPPING = {
primary:
"tracking-widest uppercase font-display bg-button-primary text-button-primary border-button-primary hover:bg-button-primary-hover hover:text-button-primary-hover hover:border-button-primary-hover",
Expand Down Expand Up @@ -55,22 +64,26 @@ const colorClasses = computed(() => {
});

const sizeClasses = computed(() => {
return SIZE_MAPPING[props.size];
return props.iconOnly ? "p-[8px]" : SIZE_MAPPING[props.size];
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible create a mapping for icon only buttons? I can imagine a few instances (in the dashboards) where we want an icon only button, but would like to have a larger button. This option would make the icon only option more useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes , i initially added it similar to the text sizing , but its not used in the current design, so i'm not sure we need it , propose we keep it simple until there is a need for this , then we can test it in the context the usecase

});

const iconPositionClass = computed(() => {
return ICON_POSITION_MAPPING[props.iconPosition];
});

const tooltipText = computed(() => {
return props.tooltip || props.iconOnly ? props.label : "";
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to make the label required if the iconOnly prop is true? Alternatively, there could be a check here that throws a warning that the label is missing or blank. Otherwise, the context is lost if the default value for label is "".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really as far as i know , but we can trow a warning i guess , i think it would mean adding a watch as the prop can change , so it might not be worth extra run time code

</script>

<template>
<button
v-tooltip.bottom="tooltipText"
class="flex items-center border rounded-input group-[.button-bar]:rounded-none group-[.button-bar]:first:rounded-l-input group-[.button-bar]:last:rounded-r-input"
:class="`${colorClasses} ${sizeClasses} ${iconPositionClass} transition-colors`"
>
<span v-if="icon">
<BaseIcon :name="icon" />
</span>
<span>{{ label }}<slot /></span>
<BaseIcon v-if="icon" :name="icon" />

<span :class="{ 'sr-only': iconOnly }">{{ label }}<slot /></span>
</button>
</template>
49 changes: 49 additions & 0 deletions apps/tailwind-components/pages/Button.story.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,54 @@
CaretDown
</Button>
</div>

<h2 class="text-2xl text-title">Icon only</h2>
<div class="flex flex-col gap-4">
<div class="flex gap-4">
<Button iconOnly icon="plus" label="add" />
<Button iconOnly icon="plus" label="add" type="secondary" />
<Button iconOnly icon="plus" label="add" type="tertiary" />
<Button iconOnly icon="plus" label="add" type="outline" />
<Button iconOnly icon="plus" label="add" type="disabled" />
<Button iconOnly icon="plus" label="add" type="filterWell" />
</div>

<div class="flex gap-4">
<Button iconOnly icon="trash" label="Remove" />
<Button iconOnly icon="trash" label="Remove" type="secondary" />
<Button iconOnly icon="trash" label="Remove" type="tertiary" />
<Button iconOnly icon="trash" label="Remove" type="outline" />
<Button iconOnly icon="trash" label="Remove" type="disabled" />
<Button iconOnly icon="trash" label="Remove" type="filterWell" />
</div>

<div class="flex gap-4">
<Button iconOnly icon="caret-up" label="previous" />
<Button iconOnly icon="caret-up" label="previous" type="secondary" />
<Button iconOnly icon="caret-up" label="previous" type="tertiary" />
<Button iconOnly icon="caret-up" label="previous" type="outline" />
<Button iconOnly icon="caret-up" label="previous" type="disabled" />
<Button iconOnly icon="caret-up" label="previous" type="filterWell" />
</div>

<div class="flex gap-4">
<Button iconOnly icon="caret-down" label="next" />
<Button iconOnly icon="caret-down" label="next" type="secondary" />
<Button iconOnly icon="caret-down" label="next" type="tertiary" />
<Button iconOnly icon="caret-down" label="next" type="outline" />
<Button iconOnly icon="caret-down" label="next" type="disabled" />
<Button iconOnly icon="caret-down" label="next" type="filterWell" />
</div>

<!-- do not render by default to avoid errors on clean load-->
<div v-if="route.query.showError" class="flex gap-4">
<!-- this should result in an error ( due to missing label)-->
<Button iconOnly icon="plus"></Button>
</div>
</div>
</div>
</template>

<script setup lang="ts">
const route = useRoute();
</script>
5 changes: 5 additions & 0 deletions apps/tailwind-components/pages/ButtonBar.story.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@
<Button icon="caret-down" size="small">three</Button>
<Button icon="trash" size="small">four</Button>
</ButtonBar>

<ButtonBar class="mt-4">
<Button :icon-only="true" icon="caret-up" label="up up up !" />
<Button :icon-only="true" icon="caret-down" label="going down" />
</ButtonBar>
</template>