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

Rename property of pagination page object to be more intuitive #13492

Closed
scrnjakovic opened this issue Sep 19, 2018 · 4 comments
Closed

Rename property of pagination page object to be more intuitive #13492

scrnjakovic opened this issue Sep 19, 2018 · 4 comments
Labels
breaks bc Functionality that breaks Backwards Compatibility enhancement Enhancement to the framework

Comments

@scrnjakovic
Copy link
Contributor

scrnjakovic commented Sep 19, 2018

I propose changing before property of page object to previous which is far more intuitive for given context, as before goes better with after, not next.

I know this is minor and breaks BC, but maybe we can do it in 4.0? Or for some time, allow both properties.

EDIT: I also think that having total_pages and last is a bit redundant. I propose removing last.

EDIT2: I'd also rename $paginator->getPaginate() to either $paginator->getPage() or $paginator->paginate() for the same reason - being more intuitive.

@niden
Copy link
Member

niden commented Sep 19, 2018

I agree with these changes, it will make things a bit more logical I think.

We can implement this right now for the next minor release but we will need to keep both versions available to keep backwards compatibility for 4.0 version as well. We can deprecate the old properties after 4.0.

In summary:

  • Introduce one more element called previous which will have the same value as before
  • Add a method called paginate() which will produce the same result as getPaginate()
  • Keep total_pages for now to be later removed since the same information is in last
  • Add @deprecated to getPaginate() to inform the developers that this will be removed after 4.0

@niden
Copy link
Member

niden commented Sep 19, 2018

@phalcon/core-team Thoughts?

@Jurigag
Copy link
Contributor

Jurigag commented Sep 19, 2018

Overall i agree. If we are going to release next minor version then we can add new methods.

@Jurigag Jurigag added enhancement Enhancement to the framework Requires-Discussion breaks bc Functionality that breaks Backwards Compatibility labels Sep 19, 2018
niden added a commit that referenced this issue Oct 10, 2018
[#13492] Changes to the paginator
niden added a commit that referenced this issue Oct 10, 2018
[#13492] Changes to the paginator
@niden
Copy link
Member

niden commented Oct 10, 2018

Implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks bc Functionality that breaks Backwards Compatibility enhancement Enhancement to the framework
Projects
None yet
Development

No branches or pull requests

3 participants