-
-
Notifications
You must be signed in to change notification settings - Fork 836
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
feat: frontend content flexible order priorities #3765
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.
Hi, thanks for the PR!
This would fix flarum/issue-archive#199
I don't know yet whether this is BC compatible or not, so need to check 🤔
@SychO9 It does indeed fix flarum/issue-archive#199 - I hadn't seen that issue up until now, glad there is a discussion for this. This is a simple PR that allows devs to have complete final control over the state of the It might be a BC like you say, as some may rely on the current behaviour of |
83fccfd
to
5e37756
Compare
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 have added priorities to the PR so that it's a lot more flexible, any developer can now choose to add content before any other content or after using priority numbers.
This allows access to page/controller loaded content when using the Frontend
content
extender - which allows developers to fully customise the document programatically, e.g. able to seeDiscussion
,Tag
, etc.Previous to this PR this was not possible:
Before -
$document
- noticetitle
is missingAfter - notice Discussion information is filled on the document and are able to access the model data.
Use Cases
This has many - one we are looking at is to add a
<meta>
SEO noindex tag to our Discussion page if we consider the posts there to have "thin content" value. With this PR, it's easily possible to get acccess to the Discussion document and relevant posts. Prior to this PR, it's not easily possible without manually loading the Discussion model, etc.Reviewers should focus on:
This potentially introduces a breaking behavioural change if people were relying on the old behaviour. I think it's a minor change though.
Necessity
Confirmed
composer test
).Required changes: