ENH Refactor: Improve code quality, remove unused code, general tidy up. #216
Conversation
GuySartorelli
force-pushed
the
pulls/4/standardise-module
branch
2 times, most recently
from
58bad22
to
4042f2f
Compare
GuySartorelli
force-pushed
the
pulls/4/standardise-module
branch
from
4042f2f
to
19848bb
Compare
GuySartorelli
force-pushed
the
pulls/4/standardise-module
branch
from
1547b72
to
1c01707
Compare
| SilverStripe\Admin\LeftAndMain: | ||
| extensions: | ||
| - SilverStripe\LinkField\Extensions\LeftAndMainExtension | ||
| extra_requirements_javascript: | ||
| 'silverstripe/linkfield:client/dist/js/bundle.js': | ||
| defer: true | ||
| extra_requirements_i18n: | ||
| - 'silverstripe/linkfield:client/lang' | ||
| extra_requirements_css: | ||
| - 'silverstripe/linkfield:client/dist/styles/bundle.css' |
There was a problem hiding this comment.
Copied from the LeftAndMainExtension - no sense having a whole extension for this. I've also raised a card to do this for other modules.
There was a problem hiding this comment.
This wasn't being used and probably doesn't even work with the updated code.
There was a problem hiding this comment.
This replaces the traits, and also includes some code that was duplicated across the two linkfield implementations.
| /** | ||
| * Set the priority of this link type in the CMS menu | ||
| */ | ||
| private static int $menu_priority = 30; |
There was a problem hiding this comment.
I updated the PHPDoc for this config property in Link - rather than trying to keep these in sync, I've opted to just remove the duplicates. API docs are smart enough to carry the description through, and for custom link types you'll probably be looking at Link rather than one of the subclasses anyway.
| use SilverStripe\Forms\CompositeValidator; | ||
| use SilverStripe\Forms\DropdownField; | ||
| use SilverStripe\Forms\FieldList; | ||
| use SilverStripe\Forms\RequiredFields; |
There was a problem hiding this comment.
Here and anywhere else I've removed use statements, it's cause they weren't being used.
| * In-memory only property used to change link type | ||
| * This case is relevant for CMS edit form which doesn't use React driven UI | ||
| * This is a workaround as changing the ClassName directly is not fully supported in the GridField admin | ||
| */ | ||
| private ?string $linkType = null; |
There was a problem hiding this comment.
This was used to let you pick a link type if creating a link in a gridfield. This isn't something we want to support, and if people really want to do it they should use GridFieldAddNewMultiClass from symbiote/silverstripe-gridfieldextensions
Most (if not all) of the other code removed in this class is related to this feature.
Requires silverstripe/silverstripe-admin#1678 to be merged for i18n stuff
Note that I have made changes in discrete commits so it's easier to review each type of change by just looking at each commit individually. Commits can be squashed if you want, though.
Issue