Favourite/star is not button

[LukasHirt]

Description

Favourite/Star button is not a button, therefore cannot receive focus

[PVince81]

I've examined a few possible approaches:

Approaches

Approach 1

• update the "oc-star" element in the design system to include a new attribute "clickable". when set, wrap the icon with a "a href" element.
• in Phoenix, set the "clickable" attribute

Approach 2

• same as approach 1 but add the "clickable" on the "oc-icon" element.

Approach 3

• simply wrap the the "oc-star" inside of Phoenix with an "a" element.

Approach 4

• change "oc-button" to also allow for removing the border

Assumptions

• sometimes we want star icons that are not clickable

Notes

• depending how far we want to go, we could also add a "pending" state to the star to provide visual feedback to fix Favorite action missing visual feedback #1903

I'm not sure yet how to decide what balance we want to have between having more logic in the design system or more in Phoenix.

@LukasHirt @felixheidecke

[PVince81]

in terms of reusability, one rule of thumb is to first add it in Phoenix (approach 3 + pending spinner) and then see later on how many times we'll want to reuse this logic.
one there is a clear requirement for reusability, refactor and move this into the design system

does that make sense ?

[LukasHirt]

I'd be actually in favour of approach 4 - button without border aka "link button". We need to adjust button component anyway to have the opportunity to use it like this also in other places + having a tag for action that doesn't lead user to any different page shouldn't be. For star that are not clickable I'd fallback to using simple oc-icon.

[LukasHirt]

There is already link button in uikit - https://getuikit.com/docs/button#style-modifiers

[PVince81]

I've tried using variation="link" on the oc-button. it is currently not allowed and we could allow for it.

however it adds an extra margin left just because there happens to be a too general rule for spans inside of buttons here: https://github.com/owncloud/owncloud-design-system/blob/master/src/styles/theme/oc-button.scss#L25

if I can solve the latter we could use "oc-button" also with the link variant

[PVince81]

Seems this is happening only because the "oc-star" is rendering two spans in the DOM, so when the second one is there it looks like there's an "oc-icon" followed by a span, to which the rule will shift the span to make room for the icon like it is done for the hamburger menu.

I'll see if I can change oc-star to only render a single element.

[PVince81]

It worked.

Now struggling with paddings, trying to make the borderless button fit in the family:

I'm still wondering whether we should rather have another element "oc-link" or something as the internal paddings will likely be different than the ones of a button.

I'll push what I have currently on a PR

[PVince81]

Draft PRs:

• Design system: Borderless buttons + using a star inside of it owncloud-design-system#481
• Phoenix: File list star is now a button #1958

[pascalwengerter]

Will get resolved by #5018

[pascalwengerter]

Fixed via #5000, coming to master soon