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.

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

Use of KExternalLink should never refer to Kolibri URLs, use KRouterLink #7482

Closed
nucleogenesis opened this issue Aug 18, 2020 · 3 comments
Closed
Labels
P2 - normal Priority: Nice to have TAG: design system Change to our style guide / design system TAG: tech update / debt Change not visible to user
Milestone

Comments

@nucleogenesis
Copy link
Member

nucleogenesis commented Aug 18, 2020

Observed behavior

The KDS KExternalLink component is supposed to be used for links outside of Kolibri. KRouterLink is supposed to link internally, even between SPAs.

Related to #7480 and the PRs/Issues that it refers to in the description.

Expected behavior

KExternalLink opens in a new tab and never links within Kolibri and every time we want this in Kolibri, we use KExternalLink
KRouterLink opens in the same tab/window and always links within Kolibri and we use KRouterLink everywhere that we link to Kolibri.

@jonboiser
Copy link
Contributor

jonboiser commented Oct 7, 2020

So, to fix this we should look for anything like this in the code:

<template>
   <KExternalLink :href="someLink">Link</KExternalLink>
</template>
<script>
import urls from 'kolibri.urls'

// Some where in the module, we're referencing something in urls:

computed: {
    someLink() {
        return urls['some:key:for:something']();
    }}
}
</script>

And replace it the KExternalLink with a KRouterLink which also supports an href prop.

@jonboiser
Copy link
Contributor

Here are some places I think this change needs to be made. These are all usages of KExternalLink that point from one Kolibri plugin SPA to another.

Sign in link on AuthMessage

<KExternalLink
:text="linkText"
:href="signInLink"
appearance="basic-link"
/>
</p>

Link from Coach -> Facility if there are no classes

<KExternalLink
v-if="isAdmin && createClassUrl"
:text="$tr('noClassesDetailsForAdmin')"
:href="createClassUrl"
/>
<span v-else>

Link from Device Settings -> Facility Settings

<KExternalLink
v-if="!isMultiFacilitySuperuser && getFacilitySettingsPath()"
:text="$tr('facilitySettings')"
:href="getFacilitySettingsPath()"
/>
</p>

<section v-if="isMultiFacilitySuperuser">
<h2>{{ $tr('configureFacilitySettingsHeader') }}</h2>
<ul class="ul-reset">
<template v-for="(facility, idx) in facilities">
<li :key="idx">
<KExternalLink
:text="facility.name"
:href="getFacilitySettingsPath(facility.id)"
/>
</li>
</template>
</ul>
</section>
</div>

Link from Facility Settings -> Device Settings

<KExternalLink
v-if="isSuperuser && deviceSettingsUrl"
:text="$tr('deviceSettings')"
:href="deviceSettingsUrl"
/>
</p>

Link to device permissions page

<KExternalLink
v-if="devicePermissionsPageLink"
class="super-admin-description"
:text="editingSelf ? $tr('viewInDeviceTabPrompt') : $tr('changeInDeviceTabPrompt')"
:href="devicePermissionsPageLink"
/>

Link from Learn page to Content import page

<KExternalLink v-if="deviceContentUrl" :text="$tr('adminLink')" :href="deviceContentUrl" />

@nucleogenesis
Copy link
Member Author

Closing in favor of a different approach, opening a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 - normal Priority: Nice to have TAG: design system Change to our style guide / design system TAG: tech update / debt Change not visible to user
Projects
None yet
Development

No branches or pull requests

2 participants