Chore/433 nav interfaces#36
Conversation
Calls to classes within Navigation.php are replaced with calls to the interface. This will allow us to have multiple implementations of the navigation while keeping the basic structure. To this end the navigation classes are renamed to illustrate that they are children of the interface parents that focus on a particular functional approach
Also update tests where classnames have changed or injection has changed from direct class to interface
RobjS
left a comment
RobjS
left a comment
There was a problem hiding this comment.
This looks good, but there's a couple of places where I think we could tighten up the interface definitions to make their intention clearer.
|
|
||
| interface ChapterNavigationInterface | ||
| { | ||
| public function getItems(): array; |
There was a problem hiding this comment.
I think we could probably tighten this up a little by specifying in a docblock/Psalm comment that this is an array of ChapterNavigationItems.
|
|
||
| interface InPageNavigationInterface | ||
| { | ||
| public function getItems(): array; |
There was a problem hiding this comment.
Similarly, here we could specify that this is an array of InPageNavigationItems.
This tightens up the logic. As a result, a new property of ChapterNavItems must be created before it can be set in Navigation
Ok makese sense, I have added that in. As a result ChapterNavItem needs a new property, which is also added |
RobjS
left a comment
RobjS
left a comment
There was a problem hiding this comment.
This all looks good to me.
One thing to note: the additional subItems property having a default value of an empty array probably means this will need to be a major version bump, because if any existing themes rely on the subItems property only existing if there are subitems, it could cause menus to output incorrect HTML.
2 participants
Description of changes
Introduces interfaces in place of navigation classes. Calls to classes within Navigation.php are replaced with calls to the interface. This will allow us to have multiple implementations of the navigation while keeping the basic structure.
To this end the navigation classes are renamed to illustrate that they are children of the interface parents that focus on a particular functional approach - in this case using page hierarchy to generate nav items.
Tests are updated and new tests added for the interfaces.
...
Checklist