Fix "Display 1 to 0 of 0 URLs" on admin list page#3910
Conversation
You know how it is - you got something perfect, a bleach-white shirt, and people will notice the tiniest speck. This one fixes the message "Display 1 to 0 of 0 URLs." on a new installation. Best regards from your friendly autist. ;-)
|
Nice catch, never noticed. This said, you hardcoded the message, making it untranslatable. See https://yourls.org/docs/development/i18n for how to make it |
OMFG you got me with my pants down (this time!) I'm very sorry for the oversight, and I hope you can find it in the bottom of your heart to forgive me. Of course it should take into account whatever i18n technique you're using. I acted rashly and was still in rage mode earlier. That said, I guess your reply means you would be rather inclined to accepting the pull if I put in some real work. :D Right now I'm on my way out, but will look into everything properly later on, cheers! |
Using yourls__ for text string now
oops, missing closing parenthesis
SimStim
left a comment
There was a problem hiding this comment.
I cannot find any .po or .mo files in the project. I guess this is just "prepared" for l10n, but there's only one language as of now? Or maybe I'm missing something..
Well, I inserted that one line I changed here into my local installation via VI and it works :) Take it or leave it.
dgw
left a comment
There was a problem hiding this comment.
Nice catch, never noticed.
The DB is initialized with a couple example links, so someone has to intentionally bring a new install down to zero stored items in order to see this :)
I cannot find any .po or .mo files in the project. I guess this is just "prepared" for l10n, but there's only one language as of now? Or maybe I'm missing something..
A .pot is maintained automatically in a separate repo: https://github.com/YOURLS/YOURLS.pot
You are correct that YOURLS itself contains only English localization. Translations based on the .pot are submitted by their maintainers to be listed on the Awesome List: https://github.com/YOURLS/awesome#translations
Message on zero URLs has been made more universal: - If no URLs in database, "No URLs." - If no URLs found while searching with filters, an additional recommendation to be less specific is added. Later it still outputs ", counting 0 clicks." - which I don't consider helpful in this particular case. But let's not lump everything into one big pull request. And the "proper" behaviour is not for me to decide anyway. So I think, my original idea "is done here."
Tab! Apply directly to the SPACES! Also more nuanced logic: only show "counting _n_ clicks" if there are more than 0 items.
|
You know, I was doing my level best to make a code review suggestion that would tweak all the style points I wanted to hit (tabs instead of spaces, whitespace around parameters, our little conventions) and then realized I should test the thing. Testing (live, in production 😱) got me thinking about more nuanced logic for display of different message parts, so I hope you don't mind that I just ended up authoring a "code review commit" directly applying the changes I'd make. Try it out! I think the output works very nicely in all cases.* * — I didn't delete all URLs from my instance to test that case, but I hit "search with results", "search with 0 results", and the default list without any filters. |
|
Sorry about the tabs and spaces, I had no idea you feel so strongly about it. You see I don't have an IDE at the moment. I edited the index.php on my server with VI, then copy pasted from the bash console into GitHub. No wonder the tabs got messed up. As for testing, you don't need to remove your urls. Just put $total_items = 0 before the if and you're good to test, even on a live system, because it doesn't affect the redirection functionality. |
dgw
left a comment
There was a problem hiding this comment.
Will let another maintainer look this over (including my tweaks).
Just put $total_items = 0 before the if and you're good to test
Ah of course, I'm clearly not in "webapp brain" today. That case is now tested too.
You see I don't have an IDE at the moment. I edited the index.php on my server with VI, then copy pasted from the bash console into GitHub. No wonder the tabs got messed up.
Funny enough, I was initially making tweaks on my server in VI. No, I don't trust its automatic tabs/spaces; final patch was checked in VS Code locally before committing.
Co-authored-by: Léo Colombaro <LeoColomb@users.noreply.github.com>
* Update index.php You know how it is - you got something perfect, a bleach-white shirt, and people will notice the tiniest speck. This one fixes the message "Display 1 to 0 of 0 URLs." on a new installation. Best regards from your friendly autist. ;-) * Update index.php Using yourls__ for text string now * Update index.php oops, missing closing parenthesis * Update index.php Message on zero URLs has been made more universal: - If no URLs in database, "No URLs." - If no URLs found while searching with filters, an additional recommendation to be less specific is added. Later it still outputs ", counting 0 clicks." - which I don't consider helpful in this particular case. But let's not lump everything into one big pull request. And the "proper" behaviour is not for me to decide anyway. So I think, my original idea "is done here." * Code feedback in the form of a commit Tab! Apply directly to the SPACES! Also more nuanced logic: only show "counting _n_ clicks" if there are more than 0 items. * check $search with empty Co-authored-by: Léo Colombaro <LeoColomb@users.noreply.github.com> --------- Co-authored-by: dgw <dgw@technobabbl.es> Co-authored-by: Léo Colombaro <LeoColomb@users.noreply.github.com>
You know how it is - you got something perfect, a bleach-white shirt, and people will notice the tiniest speck. This one fixes the message "Display 1 to 0 of 0 URLs." on a new installation. Best regards from your friendly autist. ;-)