Problem/Motivation
Hi!
After the field_type_category API has landed, core/core.field_type_category.yml gefines the category name "date_time" for some fields.
Then, both:
- TimestampItem.php (Drupal\Core\Field\Plugin\Field\FieldType),
- DateTimeItem.php (datetime\Plugin\Field\FieldType),
- and DateRangeItem.php (datetime_range\Plugin\Field\FieldType)
in their annotation it is used category = "date_time",.
However, in the module datetime_range, in its .module file we find:
/**
* Implements hook_field_type_category_info_alter().
*/
function datetime_range_field_type_category_info_alter(&$definitions) {
// The `datetime_range` field type belongs in the `general` category, so the
// libraries need to be attached using an alter hook.
$definitions[FieldTypeCategoryManagerInterface::FALLBACK_CATEGORY]['libraries'][] = 'datetime_range/drupal.datetime_range-icon';
}
using the wrong category and it should be 'date_time' instead, so the whole hook is not needed and we can remove it (commenting it does not make any impact in the 'add field' page of any content type) from the .module file and also the library and css involved as they are not used anymore.
In addition, if we take a look at FieldStorageAddForm() (line 346), we have:
$form['#attached']['library'] = [
'field_ui/drupal.field_ui',
'field_ui/drupal.field_ui.manage_fields',
'core/drupal.ajax',
];
and in the library 'field_ui/drupal.field_ui.manage_fields that calls field_ui/css/field_ui.icons.theme.css we can find the proper css that is used for the fields that are grouped under the category 'date_time':
.field-icon-date_time {}
In the same file, we find as well
.field-icon-daterange {}
but it is not used anymore as 'date_time' is the category in all the fields and can be removed too.
Steps to reproduce
Comment the whole function in the .module, then CR and see the icon is still there (in the "add new field" page under any CT bundle admin/structure/types/manage/<bundle>/fields/add-field); therefore any mention to 'daterange' css icon is not used.
Proposed resolution
- Remove the datetime_range_field_type_category_info_alter() on the .module file
- Remove the datetime_range.libraries.yml file as it it might not be needed.
- Remove the css folder, as the files inside are not needed (duplicated)
- Update the Drupal\KernelTests\Core\Theme\Stable9LibraryOverrideTest class and remove datetime_range/drupal.datetime_range-icon from there.
- Remove the .field-icon-daterange from field_ui/css/field_ui.icons.theme.css as it is not used either.
Best
Issue fork drupal-3409663
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
cilefen commentedComment #5
sandeepsingh199 commentedComment #6
gorkagr commentedHi!
You just changed the description but not the actual array value.
Edit:
Actually, it seems the whole hook function could be removed as it seems the icon and the class are duplicated and they are already added (you can check this by commenting the whole function datetime_range_field_type_category_info_alter() )
In FieldStorageAddForm() (line 346), we have:
and in the library
'field_ui/drupal.field_ui.manage_fieldsthat callsfield_ui/css/field_ui.icons.theme.csswe can find the sameas in the datetime_range.icon.theme.css
So, the proposal could be:
- Remove the datetime_range_field_type_category_info_alter() on the .module file
- Remove the .libraries file as it it might not be needed.
- Remove the css folder, as they are not needed
- Update the
Drupal\KernelTests\Core\Theme\Stable9LibraryOverrideTestclass and removedatetime_range/drupal.datetime_range-iconfrom there.Comment #7
gorkagr commentedComment #8
gorkagr commentedComment #9
gorkagr commentedComment #10
gorkagr commentedHi, sorry for all the comment and edits
I have also noticed the css it is used for rendering the icon
field-icon-date_time, not evenfield-icon-daterange, as it takes the category for the rendering due to the new panel.Comment #12
sakthi_dev commentedHi @gorkagr, updated the code based on comment #6. Should I change the array value or should it stay with 6th comment.
Comment #13
gorkagr commentedThat is more a question for a core maintainer
Comment #14
gorkagr commentedComment #15
smustgrave commentedMR Seems to have some failures.
Comment #16
gorkagr commentedComment #17
smustgrave commentedMR should be updated 11.x as the current development branch.
Looking at it though, I think it's the opposite and css should be removed from field_ui to be consistent with how other contrib modules have theirs. But since range is now under date_time the icon doesn't appear regardless so icon should be removed through out.
Comment #19
sakthi_dev commentedRebased with 11.x and also removed the daterange icon from field_ui and also removed the icon. Please review.
Comment #22
smustgrave commentedRemoval seems complete.
Comment #24
larowlanWe seem to be removing the CSS from both the field ui and the datetime range modules in the latest MR.
That doesn't seem right.
Can we update the issue summary and remove the old approach/EDIT comments so that there's a clear statement of the issue and what the proposed resolution is.
Thanks
Comment #25
gorkagr commentedComment #26
gorkagr commentedIS updated
Comment #27
smustgrave commentedIssue summary has been updated. Essentially this is dead code now as datarange isn't it's own category anymore.
Comment #28
larowlanIssue credits
Comment #31
larowlanCommitted to 11.x and backported to 10.2.x to match #3401464: Date range should be in the date_time category
Thanks folks