Show 'Page 1 of 1' instead of 'Page 1 of 0'.

[devdanzin]

This trivial PR intends to fix #1412 and plone/Products.CMFPlone#3591 by checking that totalPages is zero and setting it to one.

[thet]

Hi, thanks for the PR!

Some remarks though:

1. We're using automatic changelog generation based on commit messages using release-it, conventional-changelog and commitlint. Please change your commit message according to:
   https://github.com/Patternslib/Patterns/blob/master/docs/developer/styleguide.md#commits-messages

If you even add Fixes: ... speficiers, than you automatically reference and close the related issues.

My suggestion:

fix(pat-structure): If there are no search results, still show "Page 1 of 1" instead "Page 1 of 0".

Fixes: plone/mockup#1412
Fixes: plone/Products.CMFPlone#3591

2. See my code suggestion. I think it's simpler and I don't know if Backbone goes unnecessarily crazy when changing some state attributes too often. My simplification assigns the value in one go.

[thet]

Suggested change

	this.state.totalPages = Math.ceil(response.total / state.pageSize);
	this.state.totalPages = this.state.totalPages === 0 ? 1 : this.state.totalPages;
	// Calculate the number of pages and set it to "1" if there are no results.
	this.state.totalPages = Math.ceil(response.total / state.pageSize) || 1;

[petschki]

Looking at the issue, I'd recomment to completely remove the pagination when there is no result in the listing. Would make more sense to me. Btw. thank you @devdanzin for contributing! We were talking briefly at the conference about the plone.protect issue. Very much appreciated!

[devdanzin]

Thank you very much for the review!

1. We're using automatic changelog generation based on commit messages

I've adapted your suggestion, please let me know if it should be improved.

2. See my code suggestion.

I tried @petschki's suggestion of not showing the pagination text and it looks better to me. Please let me know which option we should go with and whether the code makes sense.

[devdanzin]

I'd recomment to completely remove the pagination when there is no result in the listing.

Done, not sure whether it's better to leave the empty div or also make it conditional. I've left it in current form.

Btw. thank you @devdanzin for contributing! We were talking briefly at the conference about the plone.protect issue.

Thank you for the review and for helping me at the sprint!

[thet]

Yes, not showing the pagination at all is even a better solution.
I lean towards not showing the div when there are no results because I favor simple, small and clean HTML output. But I'm OK with the current solution.