button, router link, external link consistency and styling

[indirectlylit]

Expected behavior

KButton, KRouterLink, and KExternalLink are styled exactly the same with 8px spacing between the icons and the text as here: #135 (comment)

Actual behavior

After merging in these PRs:

• <KExternalLink> final fixes for icons and margins #143
• Add purple clickable icons for KRouterLink #150
• KButton - basic-link: add spacing between link and icons and change color of icons #161

I see:

for:

<KExternalLink
  text="external (google)"
  href="https://google.com"
  icon="facility"
  iconAfter="facility"
/>
<br>
<KRouterLink
  text="router link (empty)"
  :to="{}"
  icon="facility"
  iconAfter="facility"
/>
<br>
<KButton
  text="button (alert)"
  appearance="basic-link"
  icon="facility"
  iconAfter="facility"
  @click="console.log('hello')"
/>

Additionally there are some discrepancies with the slots:

• KButton has icon, default, and iconAfter which seems correct
• KExternalLink is missing default and includes a slot called openInNewTab whose purpose is unclear
• KRouterLink has no slots at all

Steps to reproduce the issue

Paste the code above into a test page

Environment

• OS: MacOS
• Browser version: Chromium and Firefox

[indirectlylit]

Another thing to watch out for is that the icon(s) and text should both fade to the hover color:

no icon hover	icon hover with CSS transition

Also a bit subtle, but the timing of the hover transition is very slow compared to other components:

[sairina]

Thanks for the information! Questions:

1. KExternalLink:openInNewTab is from this issue, KExternalLink ought to open in new tab safely. From my understanding, providing true to this prop allows that link to open in another tab, instead of in the current tab. How might we adjust this to make sure it's more clear?
2. KButton: What is the purpose of default? It is this one, correct?

https://github.com/learningequality/kolibri-design-system/blob/v0.2.x/lib/buttons-and-links/KButton.vue#L23

[indirectlylit]

1. KExternalLink:openInNewTab is from this issue, KExternalLink ought to open in new tab safely. From my understanding, providing true to this prop allows that link to open in another tab, instead of in the current tab. How might we adjust this to make sure it's more clear?

Yes the purpose of the prop named openInNewTab is totally clear:

kolibri-design-system/lib/buttons-and-links/KExternalLink.vue

Lines 70 to 76 in 4a2ae39

	/**
	* If provided, opens link in new tab and displays a "pop out" icon
	*/
	openInNewTab: {
	type: Boolean,
	default: false,
	},

I'm referring to the slot named openInNewTab which was probably added by mistake:

kolibri-design-system/lib/buttons-and-links/KExternalLink.vue

Lines 32 to 39 in 4a2ae39

	<slot name="openInNewTab">
	<KIcon
	v-if="openInNewTab"
	icon="openNewTab"
	style="top: 4px;"
	:color="hovering ? $themeTokens.primaryDark : $themeTokens.primary"
	/>
	</slot>

2. KButton: What is the purpose of default? It is this one, correct?

The default slot allows you to pass arbitrary sub-components into the button, which provides more flexibility than and takes precedence over the text prop

[indirectlylit]

@sairina FYI I found the source of some of the frustrating underline issues we've had:

learningequality/kolibri#8907