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

KRouterLink to Studio: Replace <router-link> in Studio with <KRouterLink> #219

Closed
nucleogenesis opened this issue Apr 13, 2021 · 18 comments
Closed
Assignees
Labels
good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome P2 - normal Priority: Nice to have product: Studio Relevant to a specific issue in Studio type: task Something that needs to be done

Comments

@nucleogenesis
Copy link
Member

nucleogenesis commented Apr 13, 2021

Product

Studio

Expected behavior

Studio should use KComponents where available to ensure compliance with our team's design principles.

Actual behavior

Studio currently uses <router-link> but we provide a KRouterLink (source) component which abides our design system's conventions.

Additional information

The search results linked here show the few places where <router-link> is used.

The API for <KRouterLink> should map basically 1-to-1 and handle all slots seamlessly.

KRouterLink & Icons

Note that the <KRouterLink> also has the icon and iconAfter slots and props (only use one) which accept a token from the Icons listed in our Design System.

So - for example - when you work on the item in @/frontend/channelList/views/Channel/ChannelItem.vue - you'll notice that there is a slot accepting an IconButton.

You may find it easier to apply the infoPrimary icon (which will be available in the 0.2.x branch if #217 is merged) roughly as <KRouterLink icon="infoPrimary" ... />.

Alternatively, you can put a <KIcon icon="info" color="$themeTokens.primary" slot="icon" /> between <KRouterLink> ... </KRouterLink> as a slot.

You're a button KRouterLink!

<KRouterLink> may also be styled as any of our Button styles (related: KRouterLink props & slots) - so if you see that the thing using the <router-link> in Studio looks like a proper button - or an IconButton - then you can insert a KDS flavored button with the props provided by that mixin.

I don't suspect that this will affect any tests as long as you apply any data-test attributes that are affixed to the router-link to the replacement KRouterLink (there is at least one in the .../treeView/index.vue component.

@nucleogenesis nucleogenesis added type: task Something that needs to be done product: Studio Relevant to a specific issue in Studio tag: beginner friendly labels Apr 13, 2021
@nucleogenesis nucleogenesis changed the title KRouterLink -> Studio: Replace <router-link> in Studio with <KRouterLink> KRouterLink to Studio: Replace <router-link> in Studio with <KRouterLink> Apr 13, 2021
@nucleogenesis nucleogenesis added the P2 - normal Priority: Nice to have label Apr 13, 2021
@MisRob MisRob added good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome and removed tag: beginner friendly labels Aug 18, 2023
@ShivangRawat30
Copy link
Contributor

I would love to work on this issue.

@MisRob
Copy link
Member

MisRob commented Sep 25, 2023

Hi @ShivangRawat30, great, thank you. I'm assigning you. Note that even though this issue is open in this repository, you will actually work in Studio https://github.com/learningequality/studio. I'd recommend opening more pull requests with reasonable amount of changes in each, especially the first one, so we can easily test and do review before you use the same approach for the rest of Studio.

@MisRob
Copy link
Member

MisRob commented Sep 25, 2023

@ShivangRawat30 Please work from unstable Studio's branch

@MisRob
Copy link
Member

MisRob commented Nov 10, 2023

There's no activity for a long time so I will unassign you for now, @ShivangRawat30. Let us know if you are still interested and we can assign you again. Thank you for all your work!

@ShivangRawat30
Copy link
Contributor

I apologize for my absence, I was occupied with my exam

@MisRob
Copy link
Member

MisRob commented Nov 10, 2023

@ShivangRawat30 No need to apologize! I only need to clean up assignments that may be obsolete once upon a time, otherwise we would accumulate issues that are assigned but not being worked on which happens quite often. No pressure at all. You're always welcome to return to it when the right comes for you :)

@BabyElias
Copy link
Contributor

Hey @MisRob ! Can I work on this issue?

@MisRob
Copy link
Member

MisRob commented Dec 12, 2023

Hi @BabyElias, yes, you can. Thank you. I'd recommend opening more pull requests with a reasonable amount of changes in each, so that we can easily test and do review.

@BabyElias
Copy link
Contributor

Yup, sure !

@BabyElias
Copy link
Contributor

BabyElias commented Dec 13, 2023

Hey!
So I was trying to setup the Studio repository in my Windows device today and encountered the following error :
image
Tried installing this dependency separately , which caused the following error :
image

Any solutions on dealing with this? Thanks!

@MisRob
Copy link
Member

MisRob commented Dec 15, 2023

Hi @BabyElias. I'm sorry to hear that. Our team doesn't use Windows for development, but I will ask my colleagues to see if they have some tips.

It seems it may have something to do with the missing C compiler. Can you have a look at https://stackoverflow.com/questions/72543735/facing-issue-while-installing-ruamel-yaml and see if some of the advices help?

@MisRob
Copy link
Member

MisRob commented Dec 15, 2023

@BabyElias I also wanted to mention I won't be able to follow-up here this year as this is my last day before few weeks of time off. Also, the whole team's availability will too soon be limited until the second week of January. So just in case the above doesn't help and we don't figure out any other recommendations, please remind yourself after January 5 in Studio GitHub Discussion Q&A and we'll try to help with your setup.

@BabyElias
Copy link
Contributor

Sure !
Thank you so much and Happy Holidays :)

@MisRob
Copy link
Member

MisRob commented Dec 15, 2023

Thank you and Happy Holidays to you too

@BabyElias
Copy link
Contributor

Hey! So I was trying to setup the Studio repository in my Windows device today and encountered the following error : image Tried installing this dependency separately , which caused the following error : image

Any solutions on dealing with this? Thanks!

This issue is finally resolved now. Tried multiple ways of running this repo on Windows system like using WSL, Docker Run and virtualenv but all failed. Windows seems to be incompatible with the Studio repo. Finally tried by using a Virtual Machine on Ubuntu OS and it worked.

@BabyElias
Copy link
Contributor

Hey!
Can someone please review the PR #4374 so that I can continue creating PRs for changes at other places?
Thank you for your time :)

@MisRob
Copy link
Member

MisRob commented Jan 9, 2024

Hi @BabyElias, I'm glad you were able to resolve the development environment issue.

I'm sorry for delayed response, I just returned from a long vacation, and the whole team had holidays too. Thank you for the pull request! We will review.

@MisRob
Copy link
Member

MisRob commented Feb 6, 2024

Looking at the search results and I don't see any no <router-link>s left in Studio, so closing. Thank you, @BabyElias!

@MisRob MisRob closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome P2 - normal Priority: Nice to have product: Studio Relevant to a specific issue in Studio type: task Something that needs to be done
Projects
None yet
Development

No branches or pull requests

5 participants