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 support for Silverstripe 5 #61

Merged
merged 2 commits into from
Feb 28, 2023

Conversation

chrispenny
Copy link
Contributor

@chrispenny chrispenny commented Feb 12, 2023

Template namespace

Breaking change

Moved templates into JonoM namespace. Update renderWith() and <% include %> references.

Fixes #60
Closes #60

Upgrade actions: Developers will need to move any custom/overriding templates into a JonoM directory (matching what is documented in the README).

BetterNavigatorExtension changes

Breaking change

SilverStripeNavigator has moved namespace in Silverstripe 5. With that being the case, I don't believe we can support Silverstripe 4 and 5 at the same time? (Though I will be happy to be wrong).

shouldDisplay() cache change

The way that this cache was being set to a field seems to throw an error with Silverstripe 5:

ERROR [Warning]: Undefined array key "_betterNavigatorShouldDisplay"
IN GET /server-error
Line 220 in /var/www/mysite/www/vendor/silverstripe/framework/src/View/ViewableData.php

Using local/mem-cache on the Extension will achieve the same result.

BetterNavigator.ss changes

In Silverstripe 5 there is now a configuration to allow/disallow trailing slashes. For the Build databases and Dev menu links, it's easier to use relative links, as we then don't need to bother with checking if trailing slashes were present or not.

Trailing slash change: silverstripe/silverstripe-framework#10538

@jonom
Copy link
Owner

jonom commented Feb 13, 2023

Thanks for working on this @chrispenny. If you have a sec while you're dealing with namespaces you might want to take a peek at this issue, otherwise I'll aim to do that before we bump the version on this module.

@chrispenny
Copy link
Contributor Author

Thanks, @jonom! I'll have a look today :D

@chrispenny
Copy link
Contributor Author

Looks to be working with the suggested namespace change.

Template for each include:
Screen Shot 2023-02-14 at 8 01 55 AM

New buttons added:
Screen Shot 2023-02-14 at 8 01 40 AM

@chrispenny
Copy link
Contributor Author

@jonom I believe this is ready for review :)

I've tested running my local in dev and test to check that the interface shows (or hides) at appropriate times, and the buttons all look to be linking correctly.

@jonom jonom merged commit f9304b3 into jonom:master Feb 28, 2023
@jonom
Copy link
Owner

jonom commented Feb 28, 2023

Awesome, thanks @chrispenny! Have tagged 6.0.0-beta1 and created a 5 branch.

@chrispenny chrispenny deleted the pulls/silverstripe-5 branch February 28, 2023 17:11
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.

Template customisation namespace is wrong
2 participants