Problem/Motivation

Follow-up from #2225399: Apply formatters and widgets to Feed base fields.

There are only small differences between the StringTextfieldWidget, StringTextareaWidget and UriWidget classes and they repeat some code that could be moved into a base class. This would make the classes cleaner and make it easier to create further string widgets.

Also UriWidget should be set as default_widget for \Drupal\Core\Field\Plugin\Field\FieldType\UriItem

Proposed Resolution

StringTextfieldWidget, StringTextareaWidget and UriWidget should inherit from a common base class.

Remaining Tasks

A StringWidgetBase class can be used for some of the shared functionality, but StringTextfieldWidget and UriWidget share more functionality than StringTextareaWidget. This means that a single base class can't be used to remove all the shared code without the child classes deleting things inherited from their parent. None of these classes are particularly large so it needs to be decided whether more small files are preferable to repeated code.

CommentFileSizeAuthor
#14 inherit_uriwidget_from-2236599-14.patch9.85 KBstephen-cox
#9 interdiff-2236599-2-9.txt6.15 KBl0ke
#9 uri-extend-string-2236599-9.patch8 KBl0ke
#2 2236599-uri-extend-string.patch2.7 KBmr.baileys
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Related issues: +#2225399: Apply formatters and widgets to Feed base fields
mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented
Status: Active » Needs review
FileSize
2236599-uri-extend-string.patch2.7 KB

Like this?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Reviewed & tested by the community

yes, awesome

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Suppose better to address #2225399-65: Apply formatters and widgets to Feed base fields in own issue

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/UriWidget.php
@@ -72,13 +51,9 @@ public function settingsSummary() {
+    unset($element['value']['#attributes']);

This makes me think this might not be a good idea

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

So StringWidget has

      '#attributes' => array('class' => array('text-full')),

which is what we're removing here.

I don't know why we have that, but if we want to keep it, maybe we can do a StringWidgetBase, so that StringWidget only extends that one to add that class.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Needs work

nw for #6, seems a way to solve

l0ke’s picture

l0ke
Primary language English
Location Ukraine
CreditAttribution: l0ke commented
Assigned: Unassigned » l0ke
l0ke’s picture

l0ke
Primary language English
Location Ukraine
CreditAttribution: l0ke commented
Status: Needs work » Needs review
Issue tags: +#ams2014contest
FileSize
uri-extend-string-2236599-9.patch8 KB
interdiff-2236599-2-9.txt6.15 KB

Added StringWidgetBase.

awilliams’s picture

awilliams CreditAttribution: awilliams commented
Issue tags: -

I am trying to re-apply the patch if its still applies from Amsterdam

awilliams queued 9: uri-extend-string-2236599-9.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: uri-extend-string-2236599-9.patch, failed testing.

awilliams’s picture

awilliams CreditAttribution: awilliams commented
Issue summary: View changes
stephen-cox’s picture

stephen-cox CreditAttribution: stephen-cox commented
Status: Needs work » Needs review
Issue tags: +Amsterdam2014
FileSize
inherit_uriwidget_from-2236599-14.patch9.85 KB

This task has become more complicated since the addition of a StringTextareaWidget class (in #1856562: Convert "Subject" and "Message" into Message base fields) to accompany the UriWidget and the StringTextfieldWidget (renamed from StringWidget) classes.

The included patch is a re-role of #9 to deal with this extra complication. There is still some repeated code in UriWidget and StringTextfieldWidget, but I'm unsure if this is enough to warrant another base class.

stephen-cox’s picture

stephen-cox CreditAttribution: stephen-cox commented
Issue summary: View changes

Updated summary to reflect recent changes in the widget classes.

andypost queued 14: inherit_uriwidget_from-2236599-14.patch for re-testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Title: Inherit UriWidget from StringWidget » Inherit UriWidget from StringTextfieldWidget
Status: Needs review » Needs work
Issue tags: +Needs issue summary update
Related issues: +#2407505: [meta] Finalize the menu links (and other user-entered paths) system

The issue needs new summary and purpose.

The idea was that all string fields have the same settings (placeholder, size) , specifically UriWidget and StringTextfieldWidget (decoupled in #2313757: Remove text_processing option from text fields, expose existing string field types as plain text in UI) are mostly the same

Also needs to take into account #2407505: [meta] Finalize the menu links (and other user-entered paths) system

Status: Needs work » Needs review

kim.pepper queued 14: inherit_uriwidget_from-2236599-14.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 14: inherit_uriwidget_from-2236599-14.patch, failed testing.

piyuesh23’s picture

piyuesh23 CreditAttribution: piyuesh23 at QED42 commented
Issue tags: -Novice

Removing Novice tag, since the issue doesn't match the pointers mentioned on https://www.drupal.org/core-mentoring/novice-tasks. Also, as per the last comment, it needs new summary & purpose.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Version: 8.0.x-dev » 8.2.x-dev
Assigned: l0ke » Unassigned
Issue tags: -beta target

Thanks @piyuesh23!

This issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging for that as well.

This could potentially be implemented in a minor with BC (depending on what the new proposal actually is), so moving to 8.2.x.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Project: Drupal core » Aggregator
Version: 9.4.x-dev » 1.x-dev
Component: aggregator.module » Code

The aggregator module has been removed from Core in 10.0.x-dev and now lives on as a contrib module. Issues in the Core queue about the aggregator module, like this one, have been moved to the contrib module queue.

bradjones1’s picture

bradjones1
Primary language English
CreditAttribution: bradjones1 at Not Vanilla, Inc. commented
Project: Aggregator » Drupal core
Version: 1.x-dev » 10.0.x-dev
Component: Code » field system

This doesn't appear to be aggregator module but a core field type widget.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Thanks, agree

Version: 10.0.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.