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.

Sign up for GitHub

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

Clean up TODOs for v5 #6845

Draft
wants to merge 2 commits into
base: v5/develop
Choose a base branch
from
Draft

Clean up TODOs for v5 #6845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87076fa
Color field: warning for deprecated notation
distantnative Dec 7, 2024
66cc9f4
`Assets::favicons()`: warnings for depreacted keys
distantnative Dec 7, 2024
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
40 changes: 22 additions & 18 deletions config/fields/color.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use Kirby\Cms\Helpers;
use Kirby\Exception\InvalidArgumentException;
use Kirby\Field\FieldOptions;
use Kirby\Toolkit\A;
Expand Down Expand Up @@ -73,30 +74,33 @@
return [];
}

$options = match (true) {
// simple array of values
// or value=text (from Options class)
if (
is_numeric($options[0]['value']) ||
$options[0]['value'] === $options[0]['text']
=> A::map($options, fn ($option) => [
'value' => $option['text']
]),
) {
// simple array of values
// or value=text (from Options class)
$options = A::map($options, fn ($option) => [
'value' => $option['text']
]);

} elseif ($this->isColor($options[0]['text'])) {
// @deprecated 4.0.0
// TODO: Remove in Kirby 6

// deprecated: name => value, flipping
// TODO: start throwing in warning in v5
$this->isColor($options[0]['text'])
=> A::map($options, fn ($option) => [
'value' => $option['text'],
// ensure that any HTML in the new text is escaped
'text' => Escape::html($option['value'])
]),
Helpers::deprecated('Color field: the text => value notation for options has been deprecated and will be removed in Kirby 6. Please rewrite your options as value => text.');

default
=> A::map($options, fn ($option) => [
$options = A::map($options, fn ($option) => [
'value' => $option['text'],
// ensure that any HTML in the new text is escaped
'text' => Escape::html($option['value'])
]);
} else {
$options = A::map($options, fn ($option) => [
'value' => $option['value'],
'text' => $option['text']
]),
};
]);
}

return $options;
}
Expand Down
11 changes: 6 additions & 5 deletions src/Panel/Assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Kirby\Panel;

use Kirby\Cms\App;
use Kirby\Cms\Helpers;
use Kirby\Cms\Url;
use Kirby\Exception\Exception;
use Kirby\Exception\InvalidArgumentException;
Expand Down Expand Up @@ -116,11 +117,7 @@ public function external(): array
}

/**
* Returns array of favicon icons
* based on config option
*
* @todo Deprecate `url` option in v5, use `href` option instead
* @todo Deprecate `rel` usage as array key in v5, use `rel` option instead
* Returns array of favicon icons based on config option
*
* @throws \Kirby\Exception\InvalidArgumentException
*/
Expand Down Expand Up @@ -161,12 +158,16 @@ public function favicons(): array
foreach ($icons as $rel => &$icon) {
// TODO: remove this backward compatibility check in v6
if (isset($icon['url']) === true) {
Helpers::deprecated('`panel.favicon` option: use `href` instead of `url` attribute');

$icon['href'] = $icon['url'];
unset($icon['url']);
}

// TODO: remove this backward compatibility check in v6
if (is_string($rel) === true && isset($icon['rel']) === false) {
Helpers::deprecated('`panel.favicon` option: use `rel` attribute instead of passing string as key');

$icon['rel'] = $rel;
}

Expand Down
36 changes: 0 additions & 36 deletions tests/Form/Fields/ColorFieldTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,6 @@ public function testOptions()
['value' => '#bbb', 'text' => 'Color b'],
['value' => '#ccc', 'text' => 'Color c']
], $field->options());

// Deprecated: name => value
$field = $this->field('color', [
'options' => [
'Color a' => '#aaa',
'Color b' => '#bbb',
'Color c' => '#ccc'
],
]);

$this->assertSame([
['value' => '#aaa', 'text' => 'Color a'],
['value' => '#bbb', 'text' => 'Color b'],
['value' => '#ccc', 'text' => 'Color c']
], $field->options());
}

public function testOptionsFromQuery()
Expand Down Expand Up @@ -134,26 +119,5 @@ public function testOptionsFromQuery()
['value' => '#bbb', 'text' => 'Color b'],
['value' => '#ccc', 'text' => 'Color c']
], $field->options());

// Deprecated: name => value
$this->app = new App([
'options' => [
'foo' => [
'Color a' => '#aaa',
'Color b' => '#bbb',
'Color c' => '#ccc'
],
]
]);

$field = $this->field('color', [
'options' => ['type' => 'query', 'query' => 'kirby.option("foo")'],
]);

$this->assertSame([
['value' => '#aaa', 'text' => 'Color a'],
['value' => '#bbb', 'text' => 'Color b'],
['value' => '#ccc', 'text' => 'Color c']
], $field->options());
}
}
68 changes: 0 additions & 68 deletions tests/Panel/AssetsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,74 +335,6 @@ public function testFaviconsWithCustomUrl(): void
$this->assertSame($base . '/favicon.svg', $favicons[2]['href']);
}

/**
* @todo Remove backward compatibility part test in v5
*/
public function testFaviconsWithRelIndex(): void
{
// array
$this->app->clone([
'options' => [
'panel' => [
'favicon' => [
'shortcut icon' => [
'type' => 'image/svg+xml',
'url' => 'assets/my-favicon.svg',
],
'alternate icon' => [
'type' => 'image/png',
'url' => 'assets/my-favicon.png',
]
]
]
]
]);

// default asset setup
$assets = new Assets();
$favicons = $assets->favicons();

// icons
$this->assertSame('shortcut icon', $favicons[0]['rel']);
$this->assertSame('/assets/my-favicon.svg', $favicons[0]['href']);
$this->assertSame('alternate icon', $favicons[1]['rel']);
$this->assertSame('/assets/my-favicon.png', $favicons[1]['href']);
}

/**
* @todo Remove backward compatibility part test in v5
*/
public function testFaviconsWithUrlOption(): void
{
// array
$this->app->clone([
'options' => [
'panel' => [
'favicon' => [
[
'rel' => 'shortcut icon',
'type' => 'image/svg+xml',
'url' => 'assets/my-favicon.svg',
],
[
'rel' => 'alternate icon',
'type' => 'image/png',
'url' => 'assets/my-favicon.png',
]
]
]
]
]);

// default asset setup
$assets = new Assets();
$favicons = $assets->favicons();

// icons
$this->assertSame('/assets/my-favicon.svg', $favicons[0]['href']);
$this->assertSame('/assets/my-favicon.png', $favicons[1]['href']);
}

/**
* @covers ::icons
*/
Expand Down