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

Empty parenthesis is displayed when list views have more than one item, but only enough items for one page. #249

Closed
vskjefst opened this Issue Oct 16, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@vskjefst
Contributor

vskjefst commented Oct 16, 2016

This is not a big issue, it's more an annoyance. I can fix it myself, but I wanted to discuss the best approach with you, since I'd like to create a pull request when I'm done.

The problem is that on search results, category pages, and tag pages (and possibly others as well), empty parenthesis is displayed whenever there are more than 1 post to be displayed, but not enough posts to fill more than one page. In other words, the pagination information "this is page X of Y" is missing, but the parenthesis is still displayed.

The reason for this is that the parenthesis enclose the pagination information in the translation string for multiple results: "Found %1$s search results for <strong>%2$s</strong> (%3$s)." The way I see it, there are three possible solutions here:

  1. Always display pagination information. This is by far the easiest approach, because there is no need to touch the translations. But it will result in strings like "this is page 1 of 1" when there is only one page.
  2. Move the parenthesis from the"Found %1$s search results for <strong>%2$s</strong> (%3$s)." to the "this is page <strong>%1$s</strong> of <strong>%2$s</strong>" translation string. This solution will result in quite a lot of work in the current translations.
  3. Remove the parenthesis from the translation string and put them in code instead. This solution will also result in quite a lot of work in the current translations. But IMHO this is the most correct approach. It feels like not having the parenthesis in the translated strings is a more robust solution as changes to the formatting of the string won't break the existing translations. In my mind, there should be no formatting in the translation texts at all, which means that <strong>-elements should also be removed. But I'm not sure how that will work in practice, perhaps it will make translating into some languages very hard.

Any thoughts on this?

@raamdev raamdev added the bug label Oct 17, 2016

@raamdev raamdev added this to the Next Release milestone Oct 17, 2016

@raamdev

This comment has been minimized.

Owner

raamdev commented Oct 17, 2016

I did a quick review of several of the translation files and all of them seem to have the parenthesis inside the this is page ... text, instead of inside the Found %1$s search results for ... text, which, as of the latest version, is wrong (see this example). So I doubt those translated strings are even working in the latest version of the theme.

While I do try hard to avoid changing string translations with each release, when it involves fixing a bug, I don't worry too much about breaking the translations because it's up to each translation maintainer to keep the translation up-to-date with each new version of the theme. (I do consider the issue you brought up here to be a bug — I've noticed it on my own site and I kept forgetting to file a report here, so thank you!)


As for how to solve this issue, it is indeed a tricky one. :-) Thanks for breaking down the options.

Always display pagination information. This is by far the easiest approach, because there is no need to touch the translations. But it will result in strings like "this is page 1 of 1" when there is only one page.

I agree, this is definitely the easiest solution. It would only involve removing the conditional in three places in inc/template-tags.php (search that file for "this is page" to see the other places).

I like this solution not only because it's the easiest, but because the more I think about it the more correct showing "(this is page 1 of 1)" feels. It feels like that maintains better consistency and helps clarify for the visitor viewing the page that there is, in fact, only 1 page here. It's one more hint that would help the reader know there's nothing else to see except what they see on that page. Hiding that information seems unnecessary to me.

In my mind, there should be no formatting in the translation texts at all, which means that <strong>-elements should also be removed. But I'm not sure how that will work in practice, perhaps it will make translating into some languages very hard.

Yes, that's true. Whenever possible, best practice is to keep markup elements out of the translated string, but that's not always possible and it's perfectly acceptable to include the markup when there's no way around it. The goal should always be to make translating the string as easy as possible and if you split a sentence up into two separate strings just to avoid markup, you make the translator's job much harder. Leaving in the markup and keeping the sentence together is always the better option. :-)


If you'd like to submit a PR for that change to remove the conditional around $pagination_info, I'll be happy to review and merge. Or if you'd like to discuss further, let me know.

@vskjefst

This comment has been minimized.

Contributor

vskjefst commented Nov 11, 2016

Thanks for the feedback. It took a good month to get this simple fix done, but life happened. You know how it is. Anyway, glad you decided to go for my first suggestion I agree with you, it's more consistent to always show the pagination information.

@raamdev raamdev closed this in #251 Nov 11, 2016

@raamdev

This comment has been minimized.

Owner

raamdev commented Nov 11, 2016

@vskjefst No worries. :-) I know how it is! Thanks again for fixing this issue. I've merged the PR and this change will go out with the next release.

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