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
NcButton: Add alignment property to change icon and text ordering #4366
Conversation
<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> |
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.
(More examples)
<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> |
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.
I think the "start-reverse" and "end" layouts should not exist though, they look off and should not be used or recommended.
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.
@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)
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.
But as discussed with Jan, we should not promote those stypes for anything but table headers.
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.
@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?
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.
…lignment Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
b54a681
to
595f044
Compare
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.
are there other applications for trailing icons in buttons? I'm not really sure we should implement this just for pagination. Those should be link anyway and we could have a very simple wrapper of the button that just does pagination for that.
What is the problem in tables?
@marcoambrosini the sorting icon should normally be on the opposite side of the text alignment, or at least on the end instead of the inline start. But allowing to align the button content using a property seemed to be the best and cleanest solution, instead of introducing a wrapper component. There are even more examples on where this could be used, as at least the "start" alignment is quite common (have left aligned content but full width of clickable area). |
☑️ Resolves
We sometimes need to change the alignment of icon and text of the button, currently this is dirty-hacked by using
:deep
selectors. Examples are the column headers on tables (file list, logreader, file picker).🖼️ Screenshots
🏁 Checklist