-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add setCurrent()
method to Page Collection
#3398
Add setCurrent()
method to Page Collection
#3398
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a bad idea as it's not limited to siblings but next/previous items in the whole collection, which can contain any page on the site.
However, there's minor room for improvement.
{ | ||
reset($this->items); | ||
|
||
while (key($this->items) !== $path && key($this->items) !== null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would get the key into a variable to speed up the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But key setting will have to be inside the loop anyway 🤔 Will see what I can come up with. Also will check how expensive this is (I know it might be 2x faster maybe, but really doubt there will be a significant difference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 9% faster. :) Generally I prefer well-written code over repeating the same thing over and over again.
while (($key = key($this->items)) !== null && $key !== $path) {
next($this->items);
}
Ping @mahagr 🙂 |
Lets us have a workaround for #531
Also was a discussion on Discourse
This new method would allow to have expected (not inversed)
prev
andnext
items of the collection in TwigEg.: