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.

Sign up for GitHub

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

Jump to bottom

Search for prev/next sibling returns Collection #3396

Open
Karmalakas opened this issue Jul 6, 2021 · 7 comments · May be fixed by #3397
Open

Search for prev/next sibling returns Collection #3396

Karmalakas opened this issue Jul 6, 2021 · 7 comments · May be fixed by #3397

Comments

@Karmalakas
Copy link
Contributor

/system/src/Grav/Common/Page/Collection.php::adjacentSibling() may return back the whole Collection if previous or next sibling is not found. All other places in such case expect to return false. system/src/Grav/Common/Page/Interfaces/PageCollectionInterface.php::adjacentSibling() defines return types PageInterface|PageCollectionInterface|false, but PageCollectionInterface should not be there as no other method expects to return it. Also naming suggests a PageInterface if found or empty value (null|false) to be returned if not found, but not a collection
@mahagr
Copy link
Member

mahagr commented Jul 6, 2021

Yup. I have no idea why it does that, but I did add PageCollectionInterface into the return values because it did that for some crazy reason.

CC @rhukster

@Karmalakas
Copy link
Contributor Author

FYI, I'm already creating a PR for this 😉

@mahagr
Copy link
Member

mahagr commented Jul 6, 2021

I don't think it will be accepted as I'm pretty sure there was a reason why I didn't fix that.

Karmalakas added a commit to Karmalakas/grav that referenced this issue Jul 6, 2021
@Karmalakas
Return false instead of collection if adjacent item not found
5bb439c 
getgrav#3396
@Karmalakas Karmalakas linked a pull request Jul 6, 2021 that will close this issue
Open
@Karmalakas
Copy link
Contributor Author

Still created 🙂 Maybe it'll be a jump-start for Grav team to investigate this

@mahagr
Copy link
Member

mahagr commented Jul 20, 2021

I checked Flex Pages collections and it already has this behavior. Though it's not really being used, but... I must have been thinking about changing the behavior already.

@Karmalakas
Copy link
Contributor Author

Yes, elsewhere it's fine. That's why I'm leaning towards fixing page collection too.

@praserx
Copy link

praserx commented Sep 26, 2023 •
edited

Hi, any progress here? I am facing the same issue as @Karmalakas and it is quite annoying. Is there any correct (simple or straightforward) way to get page siblings?

For me, it looks like it is broken, or I am wrong? Or I don't get something important?

Edited: Sorry, bad output during debugging - collections and children itself are ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Return false instead of Collection if adjacent item not found Karmalakas/grav
3 participants
@mahagr @Karmalakas @praserx