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

Add ability to manually override templates in PageIndex #8371

Merged
merged 2 commits into from
Dec 28, 2024

Conversation

live627
Copy link
Contributor

@live627 live627 commented Dec 24, 2024

This will allow us to remove "Pages" from inline topic listings in the messageinex

Copy link
Member

@Sesquipedalian Sesquipedalian left a comment

Choose a reason for hiding this comment

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

Suggested changes:

  1. $templateOverrides$template_overrides, merely for consistency with style used elsewhere (i.e. underscores for variables, camel case for methods).
  2. Call $this->setTemplateOverrides() from constructor rather than duplicating code.
  3. For all cases, set the default value of $template_overrides parameter to [] and set the allowed type to array rather than ?array. Passing an empty array to $this->setTemplateOverrides() is harmless, so simpler and more consistent is better.

Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
Sources/PageIndex.php Outdated Show resolved Hide resolved
@Sesquipedalian Sesquipedalian merged commit 5a0150e into SimpleMachines:release-3.0 Dec 28, 2024
6 checks passed
@live627 live627 deleted the pages branch December 29, 2024 04:01
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.

2 participants