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

Add KTextTruncator component #450

Closed
3 tasks
MisRob opened this issue Sep 22, 2023 · 17 comments
Closed
3 tasks

Add KTextTruncator component #450

MisRob opened this issue Sep 22, 2023 · 17 comments
Assignees
Labels
Component: KTextTruncator good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome

Comments

@MisRob
Copy link
Member

MisRob commented Sep 22, 2023

Product

Kolibri, Studio

Summary

In Kolibri, we have TextTruncatorCss component. It would be useful in Studio as well (learningequality/studio#4086, learningequality/studio#4166).

Move TextTruncatorCss to this repository and rename it to KTextTruncator. Create a draft documentation page with some basic guidance on using it. Additionally, make sure that KTextTruncator works in RTL (right-to-left) languages (you can see learningequality/studio#4288 for how to achieve it)

Acceptance criteria

  • It supports RTL
  • It is sucessfuly used in at least one place in Kolibri
  • (can be done as follow-up) It has a draft documentation page

Outside of scope

There is no need to be replacing all TextTruncatorCss occurrences in Kolibri by KTextTruncator in the scope of this issue, only one is needed though to test that everything is okay. Neither it needs to be used in Studio. These could be follow-up tasks.

@MisRob MisRob added good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome Component: KTextTruncator Component: A new component labels Sep 22, 2023
@MisRob
Copy link
Member Author

MisRob commented Sep 22, 2023

This is already reserved for contribution for https://github.com/muditchoudhary

@muditchoudhary
Copy link
Contributor

@MisRob Okay I understand the issue. Could you please assign it to me?

@MisRob
Copy link
Member Author

MisRob commented Sep 25, 2023

Hi @muditchoudhary, great, it's all yours, thank you

@MisRob
Copy link
Member Author

MisRob commented Sep 25, 2023

@muditchoudhary It'd be best if you started working in this repository and as soon as you feel you're ready for

It is sucessfuly used in at least one place in Kolibri

please let me know and I will provide a bit of guidance as it might be tricky for the first time due to some problems with yarn link we recently discovered.

@muditchoudhary
Copy link
Contributor

It'd be best if you started working in this repository and as soon as you feel you're ready for

Yeah, sure! I have set up the local env. no issue with it.

please let me know and I will provide a bit of guidance as it might be tricky for the first time due to some problems with yarn link we recently discovered.

Sure, I'll let you know if I get into some issue that I cannot resolve. Thanks for telling about the yarn issue.

@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

@muditchoudhary Okay. I realized we even have an issue where you can find out the description of problems with yarn link (there's also a workaround with yarn run build and yarn run python-devserver mentioned there): learningequality/kolibri#10809

There's also another pretty simple workaround but let's look at it in case you can't move on with above.

@muditchoudhary
Copy link
Contributor

Hello @MisRob I have copied the TextTruncator.vue into the lib but there are two dependency functions that are in kolibri but not in kolibri-design-system.

  import responsiveElementMixin from 'kolibri.coreVue.mixins.responsiveElementMixin';
  import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';

responsiveElementMixin is imported from KResponsiveElementMixin which is present in kolibri-design So, I think I can get this in kolibri-design but commonCoreStrings is only present in kolibri.

So how do I get this commonCoreStrings into kolibri-design?

I have tried to find other issues for reference where someone added or updated a component but did not find any.

@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

Hello @muditchoudhary, we're moving TextTruncatorCss, not TextTruncator

@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

TextTruncator is not in the codebase anymore

@MisRob
Copy link
Member Author

MisRob commented Sep 27, 2023

It's been removed pretty recently so perhaps it was still on your local?

@muditchoudhary
Copy link
Contributor

Hello @muditchoudhary, we're moving TextTruncatorCss, not TextTruncator

Ohh yeah! haha I got confused. Thank you!

I did not sync my local kolibri after PR.

@muditchoudhary
Copy link
Contributor

muditchoudhary commented Oct 4, 2023

Hello @MisRob I'm almost done with solving the issue, but there is a minor issue when I'm testing the KTextTruncator on Kolibri app. However, that issue does not come when trying in kolibri-design-playground.

The ellipsis is added a little forward from the last word.

With TextTruncatorCss With KTextTruncator
image image

Tested on: Monitor with 1024x768 size

What I think could be the issue

With KTextTruncator there is some right padding but with TextTruncatorCss there is no right padding.

With TextTruncatorCss With KTextTruncator
image image

I used yarn link process to install this KTextTruncator into Kolibri.
RTL languages works fine but the same issue is also present in Arabic

@MisRob
Copy link
Member Author

MisRob commented Oct 6, 2023

Hi @muditchoudhary, thanks for staying in touch, this is helpful.

I think you can feel free to adjust KTextTruncator code to work as expected in Kolibri (that is, without the gap)

@MisRob
Copy link
Member Author

MisRob commented Oct 6, 2023

Also, you could see if there are some styles in Kolibri that could be causing this unexpectedly and will need to be removed as we're introducing KTextTruncator. However as mentioned, introducing it at all places in Kolibri is not in the scope of this issue (could be done as a follow-up) so testing just a few places and ensuring it works reliably there is sufficient.

@MisRob
Copy link
Member Author

MisRob commented Oct 6, 2023

Let me know if this helps. If not, you could open a draft PR with your work in progress and we can have a look at code together

@MisRob
Copy link
Member Author

MisRob commented Oct 23, 2023

Closed by #464

@MisRob MisRob closed this as completed Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: KTextTruncator good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome
Projects
None yet
Development

No branches or pull requests

2 participants