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

make ui menu accept maxWidth to avoid text truncation #870

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

lokesh-sagi125
Copy link
Contributor

Description

override the max-width set by this component UiMenu. And for this, we will need to update our KDropdownMenu component to accept a new prop maxWidth that will then be passed to the UiMenu component.

Issue addressed

learningequality/kolibri#5557

Addresses learningequality/kolibri#5557

Changelog

@lokesh-sagi125 lokesh-sagi125 changed the title remove text truncation make ui menu accept maxWidth to avoid text truncation Dec 11, 2024
@MisRob
Copy link
Member

MisRob commented Dec 12, 2024

Hi @lokesh-sagi125, would you upload some screenshots of how the output would look like for learningequality/kolibri#5557? Also, what changes you did in Kolibri after having this KDS update available to achieve that - do you have a Kolibri working branch you could reference?

At first sight, I don't understand the connection of this work to the problem with the file size being pushed out - this would help me to connect the dots.

@MisRob
Copy link
Member

MisRob commented Dec 12, 2024

Oh I just noticed @AlexVelezLl's #858 (comment). Alright, it seems fine (generally I'd recommend you always upload screenshots to pull requests so reviewers don't need to do the detective work :)

One question I had is if the title part gets truncated? It's correct that the title takes less space so the file size is always visible, but if the whole of the title is not visible, we should still show the ellipsis ...

@MisRob MisRob requested review from MisRob and AlexVelezLl December 12, 2024 04:21
@MisRob MisRob added the TODO: needs review Waiting for review label Dec 12, 2024
@AlexVelezLl
Copy link
Member

Oh @MisRob, In this case we should show the whole string without it being truncated because this string is not a user string, but a system string. In this context the string is showing the different files from a resource that can be downloaded (e.g. "Document", "thumbnail", "HTML"). So, if we truncate this string, the message could be a little confusing, and the user might not be able to understand what is they downloading, so thats why Blaine suggested that the better option here is to show the full message and avoid truncating a system string:

If the text isn't user generated, I don't think there should be text truncation on that at all, because that calls into question the usefulness of the text, if it's being truncated in some languages.

@MisRob
Copy link
Member

MisRob commented Jan 14, 2025

I see @AlexVelezLl, so can I understand right that the string will take more lines in the case you describe? I think that'd be fine, the main thing is that the whole text is visible.

@AlexVelezLl
Copy link
Member

AlexVelezLl commented Jan 15, 2025

Hmm no @MisRob, the long string will be all placed in just one line like this, but will not get truncated:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants