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

Jump to bottom

NcButton: Add alignment property to change icon and text ordering #4366

Merged
merged 1 commit into from Aug 2, 2023
Merged

NcButton: Add alignment property to change icon and text ordering #4366

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 78 additions & 1 deletion src/components/NcButton/NcButton.vue
View file
Open in desktop
Expand Up @@ -197,6 +197,41 @@ button {
</style>
```

### Alignment
Sometimes it is required to change the icon alignment on the button, like for switching between pages as in following example:

```vue
<template>
<div>
<div style="display: flex; gap: 12px;">
<NcButton aria-label="Previous" type="tertiary">
<template #icon>
<IconLeft :size="20" />
</template>
Previous page
</NcButton>
<NcButton alignment="end-reverse" aria-label="Previous" type="primary">
<template #icon>
<IconRight :size="20" />
</template>
Next page
</NcButton>
</div>
</div>
Comment on lines +205 to +220
Copy link
Contributor

Choose a reason for hiding this comment

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

(More examples)

Suggested change
<div>
<div style="display: flex; gap: 12px;">
<NcButton aria-label="Previous" type="tertiary">
<template #icon>
<IconLeft :size="20" />
</template>
Previous page
</NcButton>
<NcButton alignment="end-reverse" aria-label="Previous" type="primary">
<template #icon>
<IconRight :size="20" />
</template>
Next page
</NcButton>
</div>
</div>
<div style="display: flex; flex-direction: column; gap: 12px;">
<NcButton aria-label="central (default)" type="secondary" wide>
<template #icon>
<IconLeft :size="20" />
</template>
central (default)
</NcButton>
<div style="display: flex; gap: 12px;">
<div style="display: flex; flex-direction: column; gap: 12px; flex: 1">
<NcButton alignment="start" aria-label="start" type="secondary" wide>
<template #icon>
<IconLeft :size="20" />
</template>
start
</NcButton>
<NcButton alignment="start-reverse" aria-label="start-reverse" type="secondary" wide>
<template #icon>
<IconRight :size="20" />
</template>
start-reverse
</NcButton>
</div>
<div style="display: flex; flex-direction: column; gap: 12px; flex: 1">
<NcButton alignment="end" aria-label="end" type="secondary" wide>
<template #icon>
<IconLeft :size="20" />
</template>
end
</NcButton>
<NcButton alignment="end-reverse" aria-label="end-reverse" type="secondary" wide>
<template #icon>
<IconRight :size="20" />
</template>
end-reverse
</NcButton>
</div>
</div>
</div>

image

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 the "start-reverse" and "end" layouts should not exist though, they look off and should not be used or recommended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jancborchardt start-reverse and end are the values for which this was feature is introduced (table headers sort icon should be on the opposite site of the alignment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But as discussed with Jan, we should not promote those stypes for anything but table headers.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jancborchardt start-reverse and end are the values for which this was feature is introduced (table headers sort icon should be on the opposite site of the alignment)

Do you have end, not end-reversed button in file picker table?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, center-reversed also may make sense for aka pagination buttons or next-step buttons with right arrow.

image

We don't have such buttons right now. But it seems to me that the main reason for this is that developers do not use icons here because they are not supported and because it is not an option.

</template>
<script>
import IconLeft from 'vue-material-design-icons/ArrowLeft.vue'
import IconRight from 'vue-material-design-icons/ArrowRight.vue'

export default {
components: {
IconLeft,
IconRight,
},
}
</script>
```

### Pressed state
It is possible to make the button stateful by adding a pressed state, e.g. if you like to create a favorite button.
The button will have the required `aria` attribute for accessibility and visual style (`primary` when pressed, and the configured type otherwise).
Expand Down Expand Up @@ -264,6 +299,17 @@ export default {
name: 'NcButton',

props: {
/**
* Set the text and icon alignment
*
* @default 'center'
*/
alignment: {
type: String,
default: 'center',
validator: (alignment) => ['start', 'start-reverse', 'center', 'end', 'end-reverse'].includes(alignment),
},

/**
* Toggles the disabled state of the button on and off.
*/
Expand Down Expand Up @@ -388,6 +434,20 @@ export default {
}
return this.type
},

/**
* The flexbox alignment of the button content
*/
flexAlignment() {
return this.alignment.split('-')[0]
},

/**
* If the button content should be reversed (icon on the end)
*/
isReverseAligned() {
return this.alignment.includes('-')
},
},

/**
Expand Down Expand Up @@ -423,6 +483,8 @@ export default {
'button-vue--icon-and-text': hasIcon && hasText,
[`button-vue--vue-${this.realType}`]: this.realType,
'button-vue--wide': this.wide,
[`button-vue--${this.flexAlignment}`]: this.flexAlignment !== 'center',
'button-vue--reverse': this.isReverseAligned,
active: isActive,
'router-link-exact-active': isExactActive,
},
Expand Down Expand Up @@ -556,6 +618,20 @@ export default {
width: 100%;
}

&--end &__wrapper {
justify-content: end;
}
&--start &__wrapper {
justify-content: start;
}
&--reverse &__wrapper {
flex-direction: row-reverse;
}

&--reverse#{&}--icon-and-text {
padding-inline: calc(var(--default-grid-baseline) * 4) var(--default-grid-baseline);
}

&__icon {
height: $clickable-area;
width: $clickable-area;
Expand Down Expand Up @@ -591,7 +667,8 @@ export default {

// Icon and text button
&--icon-and-text {
padding: 0 16px 0 4px;
padding-block: 0;
padding-inline: var(--default-grid-baseline) calc(var(--default-grid-baseline) * 4);
}

// Wide button spans the whole width of the container
Expand Down