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

Fix Page Collections problem with @page.modular #1178

Merged
merged 1 commit into from
Dec 2, 2016

Conversation

haagvivi
Copy link
Contributor

Loading the page children before call modular on the Collection.

In order to view the complete example issue you can refer to https://getgrav.org/forum#!/general:cant-get-modular-collectio

@rhukster
Copy link
Member

Thanks I'll look at this soon and review.

Copy link
Contributor

@flaviocopes flaviocopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and replicated the problem, the PR is fixing it.

@mahagr
Copy link
Member

mahagr commented Nov 22, 2016

There are two kinds of children: modular and non-modular. After the fix both will be returned, which is not correct.

Copy link
Member

@mahagr mahagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try creating a modular page with sub-menu in it. You really need to add a test for modular item before adding the child pages.

@flaviocopes
Copy link
Contributor

I tried adding a non-modular child, it's not picked up, as it's calling $results->modular();. Not sure if I "got" the problem you notice

Copy link
Member

@mahagr mahagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right, I was blind.

@haagvivi
Copy link
Contributor Author

Yes, the modular function juste keep the modular pages.

@rhukster rhukster merged commit 10da784 into getgrav:develop Dec 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants