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

changed column three dots to ellipsis #3993

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Conversation

tabacitu
Copy link
Member

@tabacitu tabacitu commented Dec 7, 2021

WHY

BEFORE - What was wrong? What was happening before this PR?

Before this PR, I was a little bit sad & bored. So I decided to make someone happy today (not saying who 😉 ).

Also, when a column was truncated, it showed [...] at the end.

AFTER - What is happening after this PR?

It shows a at the end instead.

HOW

How did you achieve that, in technical terms?

Find & replace.

Is it a breaking change or non-breaking change?

Non-breaking.

How can we test the before & after?

You don't:

2021-12-07 11 07 19

@tabacitu tabacitu requested a review from promatik December 7, 2021 09:13
@tabacitu tabacitu merged commit 84394eb into 4.2 Dec 7, 2021
@tabacitu tabacitu deleted the columns-dots-to-ellipsis branch December 7, 2021 09:13
@tabacitu tabacitu mentioned this pull request Dec 7, 2021
@pxpm
Copy link
Contributor

pxpm commented Dec 7, 2021

@tabacitu hahaha Santa is coming earlier this year :)

I am going to be the little green elf that will try to ruin the christmas this year, but when I see this kind of changes I think it would be better if this was a possible configuration of the columns, instead of changing one opinionated value for another one. I am not sure if ... indicates better than [...] that the text had been truncated. Depending on the content stored in the column the three dots could look more like part of a regular person writting than the brackets with dots. I've also seen (...) beeing used.

But probably there is some gramatical rule about it that I don't know, but beeing part of column configuration wouldn't hurt I guess.

Pedro

@promatik
Copy link
Contributor

promatik commented Dec 7, 2021

Yeah 🎉 😂

@tabacitu something that I haven't mention, with the Laravel-Backpack/editable-columns, since it uses <input>, it behaves different from the other default columns with <span> and <p>, so I added the text-overflow: ellipsis; there, to get this result;

image

This PR makes everything even 🙌
is also the standard on HTML/CSS, so everywhere 🎉

@tabacitu
Copy link
Member Author

tabacitu commented Dec 8, 2021

I am going to be the little green elf that will try to ruin the christmas this year

😅

I think it would be better if this was a possible configuration of the columns, instead of changing one opinionated value for another one

I totally agree. It should be a config. But when I did a search & replace I found that not all columns have a limit, so... doing it right would probably entail a lot more time spent on this, evaluating which columns should have it and which shouldn't... talking about WHEN and HOW to do that... bleah. We definitely don't have time to do that right now, since this is a COULD. So I just did a quick&dirty find&replace, that at least cleans things up visually a bit. I didn't mind the brackets either, but @promatik had a pretty big problem with them 😀

I've added an issue for this, to maybe make it configurable in 4.2.x or v12.x - (done here - Laravel-Backpack/community-forum#119 ). But the fact and the matter is... nobody asked for this yet. If nobody asked to make it configurable... we probably shouldn't. Otherwise... where do we stop?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants