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

ENH Add sorting by link type #167

Merged
merged 1 commit into from
Jan 11, 2024
Merged

ENH Add sorting by link type #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions lang/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ en:
INVALID_TYPECLASS_EMPTY: '"{class}": Allowed types cannot be empty'
SilverStripe\LinkField\Models\EmailLink:
EMAIL_FIELD: 'Email address'
LINKLABEL: 'Link to email address'
PLURALNAME: 'Email Links'
PLURALS:
one: 'An Email Link'
Expand All @@ -26,6 +27,7 @@ en:
db_Email: Email
SilverStripe\LinkField\Models\ExternalLink:
EXTERNAL_URL_FIELD: 'External url'
LINKLABEL: 'Link to external URL'
PLURALNAME: 'External Links'
PLURALS:
one: 'An External Link'
Expand All @@ -36,6 +38,7 @@ en:
CANNOT_VIEW_FILE: 'Cannot view file'
FILE_DOES_NOT_EXIST: 'File does not exist'
FILE_FIELD: File
LINKLABEL: 'Link to a file'
MISSING_DEFAULT_TITLE: 'File missing'
PLURALNAME: 'File Links'
PLURALS:
Expand All @@ -59,6 +62,7 @@ en:
db_Version: Version
has_one_Owner: Owner
SilverStripe\LinkField\Models\PhoneLink:
LINKLABEL: 'Phone number'
PHONE_FIELD: Phone
PLURALNAME: 'Phone Links'
PLURALS:
Expand All @@ -70,6 +74,7 @@ en:
ANCHOR_DESCRIPTION: 'Do not prepend "#". Anchor suggestions will be displayed once the linked page is attached.'
ANCHOR_FIELD_TITLE: Anchor
CANNOT_VIEW_PAGE: 'Cannot view page'
LINKLABEL: 'Page on this site'
MISSING_DEFAULT_TITLE: 'Page missing'
PAGE_DOES_NOT_EXIST: 'Page does not exist'
PAGE_FIELD_TITLE: Page
Expand Down
7 changes: 5 additions & 2 deletions src/Form/Traits/AllowedLinkClassesTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,14 @@ public function getTypesProps(): string
}
$typesList[$key] = [
'key' => $key,
'title' => $type->i18n_singular_name(),
'title' => $type->getMenuTitle(),
'handlerName' => $type->LinkTypeHandlerName(),
'priority' => $class::config()->get('menu_priority'),
];
}
uasort($typesList, function ($a, $b) {
return $a['priority'] - $b['priority'];
});
Comment on lines +101 to +103
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uasort($typesList, function ($a, $b) {
return $a['priority'] - $b['priority'];
});
uasort($typesList, fn ($a, $b) => $a['priority'] - $b['priority']);

I won't block merging the PR for this - but in the future we should use the short function syntax for this sort of thing, since we're just returning a value.
The long function syntax is only really valuable when we've got multiple lines of functionality or when the return statement is really long.


return json_encode($typesList);
}
Expand All @@ -121,7 +125,6 @@ private function genarateAllowedTypes(): array
$result[$type] = $class;
}
}

return $result;
}
}
14 changes: 14 additions & 0 deletions src/Models/EmailLink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ class EmailLink extends Link
'Email' => 'Varchar(255)',
];

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 30;

public function getCMSFields(): FieldList
{
$this->beforeUpdateCMSFields(function (FieldList $fields) {
Expand All @@ -38,4 +43,13 @@ public function getURL(): string
{
return $this->Email ? sprintf('mailto:%s', $this->Email) : '';
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
*/
public function getMenuTitle(): string
{
return _t(__CLASS__ . '.LINKLABEL', 'Link to email address');
}
}
14 changes: 14 additions & 0 deletions src/Models/ExternalLink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class ExternalLink extends Link
'ExternalUrl' => 'Varchar',
];

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 20;

public function getCMSFields(): FieldList
{
$this->beforeUpdateCMSFields(function (FieldList $fields) {
Expand All @@ -35,4 +40,13 @@ public function getURL(): string
{
return $this->ExternalUrl ?: '';
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
*/
public function getMenuTitle(): string
{
return _t(__CLASS__ . '.LINKLABEL', 'Link to external URL');
}
}
14 changes: 14 additions & 0 deletions src/Models/FileLink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ class FileLink extends Link
'File' => File::class,
];

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 10;

public function getCMSFields(): FieldList
{
$this->beforeUpdateCMSFields(function (FieldList $fields) {
Expand Down Expand Up @@ -55,4 +60,13 @@ public function getDefaultTitle(): string

return (string) $this->getDescription();
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
*/
public function getMenuTitle(): string
{
return _t(__CLASS__ . '.LINKLABEL', 'Link to a file');
}
}
16 changes: 16 additions & 0 deletions src/Models/Link.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class Link extends DataObject
*/
private ?string $linkType = null;

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 100;

public function getDescription(): string
{
return '';
Expand Down Expand Up @@ -454,4 +459,15 @@ public function getShortCode(): string
{
return strtolower(rtrim(ClassInfo::shortName($this), 'Link')) ?? '';
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
* Subclasses should override this.
* It will use the singular_name by default.
*/
public function getMenuTitle(): string
{
return $this->i18n_singular_name();
}
}
14 changes: 14 additions & 0 deletions src/Models/PhoneLink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ class PhoneLink extends Link
'Phone' => 'Varchar(255)',
];

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 40;

public function getCMSFields(): FieldList
{
$this->beforeUpdateCMSFields(function (FieldList $fields) {
Expand All @@ -33,4 +38,13 @@ public function getURL(): string
{
return $this->Phone ? sprintf('tel:%s', $this->Phone) : '';
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
*/
public function getMenuTitle(): string
{
return _t(__CLASS__ . '.LINKLABEL', 'Phone number');
}
}
14 changes: 14 additions & 0 deletions src/Models/SiteTreeLink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ class SiteTreeLink extends Link
'Page' => SiteTree::class,
];

/**
* Set the priority of this link type in the CMS menu
*/
private static int $menu_priority = 0;

public function getDescription(): string
{
$page = $this->Page();
Expand Down Expand Up @@ -131,4 +136,13 @@ public function getDefaultTitle(): string
}
return $page->Title;
}

/**
* The title that will be displayed in the dropdown
* for selecting the link type to create.
*/
public function getMenuTitle(): string
{
return _t(__CLASS__ . '.LINKLABEL', 'Page on this site');
}
}
73 changes: 73 additions & 0 deletions tests/php/Traits/AllowedLinkClassesTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use ArrayIterator;
use InvalidArgumentException;
use ReflectionMethod;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\LinkField\Form\Traits\AllowedLinkClassesTrait;
use SilverStripe\LinkField\Form\LinkField;
Expand Down Expand Up @@ -105,6 +106,78 @@ public function provideTypesExceptionDataProvider() : array
];
}

public function sortedTypesDataProvider() : array
{
return [
'sort all allowed Link classes' => [
'enabled' => [
SiteTreeLink::class,
ExternalLink::class,
FileLink::class,
EmailLink::class,
PhoneLink::class,
TestPhoneLink::class,
],
'expected' => [
'sitetree',
'file',
'external',
'email',
'phone',
'testphone',
],
'reorder' => false,
],
'sort all allowed Link classes and move TestPhoneLink up ' => [
'enabled' => [
SiteTreeLink::class,
ExternalLink::class,
FileLink::class,
EmailLink::class,
PhoneLink::class,
TestPhoneLink::class,
],
'expected' => [
'sitetree',
'testphone',
'file',
'external',
'email',
'phone',
],
'reorder' => true,
],
'sort only particular allowed Link class and move TestPhoneLink up' => [
'enabled' => [
SiteTreeLink::class,
TestPhoneLink::class,
EmailLink::class,
],
'expected' => [
'sitetree',
'testphone',
'email',
],
'reorder' => true,
],
];
}

/**
* @dataProvider sortedTypesDataProvider
*/
public function testGetTypesProps(array $enabled, array $expected, bool $reorder): void
{
if ($reorder) {
Injector::inst()->get(TestPhoneLink::class)->config()->set('menu_priority', 5);
}

$linkField = LinkField::create('LinkField');
$linkField->setAllowedTypes($enabled);
$json = json_decode($linkField->getTypesProps(), true);
$this->assertEquals(array_keys($json), $expected);
}

public function testGetTypesPropsCanCreate(): void
{
$linkField = LinkField::create('LinkField');
Expand Down