Borderless buttons + using a star inside of it

[PVince81]

Allow the "link" variation on oc-button for borderless buttons. Note
that buttons are needed for accessibility tools.
Fix oc-star to not render two spans as it would trigger the CSS rule
".oc-button .oc-icon + span" for the second span which would add an
unwanted left margin.

This is for owncloud/web#1645.

[CLAassistant]

All committers have signed the CLA.

[PVince81]

It seems having a borderless "oc-button" is looking for trouble: owncloud/web#1645 (comment)

It would require defining a special padding for when it's used only as a link.

Since this is visually very different we might rather need to provide another variant, maybe an "oc-link"

[PVince81]

@LukasHirt see comment above and let me know what you think

[PVince81]

it seems that "oc-icon" is not using UIKit's icons, so we cannot use the link modifier from https://getuikit.com/docs/icon#link-modifier

[LukasHirt]

@PVince81 paddings should be stored somewhere in variables so we just need to check which exactly are the buttons using and bring it also into the link button. For deciding if we want to have the padding there or not I’d stick with prop which would be assigning class with the padding.

In regards to icons - yes, we’re not using the UIkit ones. Instead we mostly use material design icons. This will be later replaced with our own ownCloud icons as soon as they’re created.

[DeepDiver1975]

Icons are Design elements which have to be defined in the Design system.
This is how a Design system works.

.... Uikit .... 🤪🤯

[PVince81]

@marcus-herrmann do you have an idea if the above properly fits in the design system in this form or whether it should be done differently ?

Please note the original ticket owncloud/web#1645 where it was about having the star clickable for accessibility reasons

[marcus-herrmann]

@PVince81

I find it problematic that this component tries to solve two use cases (at least as far as I understand the code examples where you differentiate between star and clickable star):

• Image representation of a star
• A toggle button

If it's okay, I would make a different suggestion, if it is permanently meant to be a control and not also an controll-less image:

• Hard-wire the button characteristics into the component
• Make the button's ariaLabel label prop mandatory, it is still an icon-only button at the end
• But actually make ariaLabel a mandatory prop for oc-star. Forcing ariaLabel as a mandatory prop to oc-button would do more harm than good, since not all buttons are icon-only buttons und you should not use aria-label on an already labelled button <button>This form of label</button>.
• Communicate its state with an aria attribute (aria-pressed), also add semantics that this is a toggle/switch button (role="switch").

I could supply my suggestion here in the form of another PR.

[PVince81]

@marcus-herrmann your proposal sounds good

[PVince81]

seems one can use variation=raw in oc-button. I missed that, closing.