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

Make ancestorClass option consistency to underscore #351

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ff8864b
Make ancestorClass option consistency to underscore
shakaran Aug 31, 2021
4fc676d
Require symfony/deprecation-contracts for trigger warnings
shakaran Aug 31, 2021
df85637
Add deprecation contracts and allow both ways for support BC
shakaran Aug 31, 2021
d2b325b
Fix if spaced as per review suggested
shakaran Aug 31, 2021
d7a3399
Revert changes in doc for deprecated way as per review suggested
shakaran Aug 31, 2021
3aeb687
Update version number for deprecation
shakaran Aug 31, 2021
24fa9c2
Improve warning message for twig deprecation as per review suggested
shakaran Aug 31, 2021
4103108
Merge branch 'master' into ancestor_class_underscore
shakaran Aug 31, 2021
05e3231
Update test for show as legacy deprecation warnings
shakaran Aug 31, 2021
5bf59b9
Avoid duplication of configs and show deprecations warnings too
shakaran Aug 31, 2021
5ae265b
Update php version in composer
shakaran Aug 31, 2021
10569aa
Try to avoid replacements by deprecation contracts in github build
shakaran Aug 31, 2021
abdeda8
Try another sed vudu way
shakaran Aug 31, 2021
ae50e57
Add exclusion pattern /pattern/!s/pattern/
shakaran Aug 31, 2021
98046aa
Update the sed vudu charm
shakaran Aug 31, 2021
4ef0026
Vudu sed require a space
shakaran Aug 31, 2021
d82c70f
Update build.yaml
garak Aug 31, 2021
4abbba4
Add support deprecation for knp_menu_render and knp_menu_get with anc…
shakaran Jan 26, 2022
9c5797c
Use ExpectDeprecationTrait for deprecation message
shakaran Jan 26, 2022
78608e3
Add @group Legacy or Legacy in name for allow pass test in CI
shakaran Jan 26, 2022
742aacc
Mistake preprend at beginning legacy
shakaran Jan 26, 2022
9de5229
Add as annotation since CI don't recognize
shakaran Jan 26, 2022
c42343b
Remove message since expects empty message for trait
shakaran Jan 26, 2022
a4bc394
Add empty string message for deprecation instead null
shakaran Jan 26, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
}
],
"require": {
"php": "^7.2 || ^8.0"
"php": "^7.2 || ^8.0",
garak marked this conversation as resolved.
Show resolved Hide resolved
"symfony/deprecation-contracts": "^2.4"
},
"conflict": {
"twig/twig": "<1.40 || >=2,<2.9"
Expand Down
2 changes: 1 addition & 1 deletion doc/01-Basic-Menus.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ the second argument to the `render()` method:
is an ancestor of the current item.
* `currentAsLink` (default: `true`): Whether to render the "current" menu item as link or as span.
* `currentClass` (default: `current`)
* `ancestorClass` (default: `current_ancestor`)
* `ancestor_class` (default: `current_ancestor`) (alias: ancestorClass, currently deprecated)
* `firstClass` (default: `first`)
* `lastClass` (default: `last`)
* `compressed` (default: `false`)
Expand Down
16 changes: 15 additions & 1 deletion doc/examples/01_apply_active_class_to_whole_tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ By default the active menu item is applied with a `current` class and it's ances
class. To apply a class other than the default or apply the same class to both you can either pass it as option to the
render method of your used renderer:

```php
<?php
$renderer->render($item, ['currentClass' => 'active', 'ancestor_class' => 'active']);
```

Old deprecated way:
garak marked this conversation as resolved.
Show resolved Hide resolved

```php
<?php
$renderer->render($item, ['currentClass' => 'active', 'ancestorClass' => 'active']);
Expand All @@ -14,5 +21,12 @@ or pass it as argument to the constructor of the used renderer:

```php
<?php
$renderer = new Renderer($matcher, ['currentClass' => 'active', 'ancestorClass' => 'active']);
$renderer = new Renderer($matcher, ['currentClass' => 'active', 'ancestor_class' => 'active']);
```

Old deprecated way:

```php
<?php
$renderer = new Renderer($matcher, ['currentClass' => 'active', 'ancestorClass' => 'active'])
```
9 changes: 7 additions & 2 deletions src/Knp/Menu/Renderer/ListRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public function __construct(MatcherInterface $matcher, array $defaultOptions = [
'matchingDepth' => null,
'currentAsLink' => true,
'currentClass' => 'current',
'ancestorClass' => 'current_ancestor',
'ancestor_class' => 'current_ancestor',
'ancestorClass' => 'current_ancestor', // Deprecated in future
'firstClass' => 'first',
'lastClass' => 'last',
'compressed' => false,
Expand Down Expand Up @@ -130,7 +131,11 @@ protected function renderItem(ItemInterface $item, array $options): string
if ($this->matcher->isCurrent($item)) {
$class[] = $options['currentClass'];
} elseif ($this->matcher->isAncestor($item, $options['matchingDepth'])) {
$class[] = $options['ancestorClass'];
if(isset($options['ancestorClass']) { // Deprecated: it will be removed in future (@see: ancestor_class)
shakaran marked this conversation as resolved.
Show resolved Hide resolved
trigger_deprecation('knplabs/knp-menu', '3.2', 'Using "%s" option is deprecated, use "%s" instead.', 'ancestorClass', 'ancestor_class');
garak marked this conversation as resolved.
Show resolved Hide resolved
$options['ancestor_class'] = $options['ancestorClass'];
}
$class[] = $options['ancestor_class'];
}

if ($item->actsLikeFirst()) {
Expand Down
2 changes: 1 addition & 1 deletion src/Knp/Menu/Renderer/RendererInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface RendererInterface
* 1: only direct children
* - currentAsLink: whether the current item should be a link
* - currentClass: class added to the current item
* - ancestorClass: class added to the ancestors of the current item
* - ancestor_class: class added to the ancestors of the current item (alias: ancestorClass, deprecated in a future)
* - firstClass: class added to the first child
* - lastClass: class added to the last child
*
Expand Down
1 change: 1 addition & 0 deletions src/Knp/Menu/Renderer/TwigRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public function __construct(
'matchingDepth' => null,
'currentAsLink' => true,
'currentClass' => 'current',
'ancestor_class' => 'current_ancestor',
'ancestorClass' => 'current_ancestor',
'firstClass' => 'first',
'lastClass' => 'last',
Expand Down
3 changes: 2 additions & 1 deletion src/Knp/Menu/Resources/views/knp_menu.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
{%- if matcher.isCurrent(item) %}
{%- set classes = classes|merge([options.currentClass]) %}
{%- elseif matcher.isAncestor(item, options.matchingDepth) %}
{%- set classes = classes|merge([options.ancestorClass]) %}
{# ancestorClass will be deprecated in future, use ancestor_class #}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this does not report the deprecation AFAICT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 24fa9c2

{%- set classes = classes|merge([options.ancestor_class])|merge([options.ancestorClass]) %}
{%- endif %}
{%- if item.actsLikeFirst %}
{%- set classes = classes|merge([options.firstClass]) %}
Expand Down