Use case: image field set to unlimited, but you only want to show the first on a teaser view.

This is now possible with a contrib module Field Limiter. We could adopt that code into Drupal Core.;

Note that instead of limiting the output, one could also limit the input for each field instance with Field Config Cardinality.

CommentFileSizeAuthor
#115 interdiff-105-115.txt1.37 KBashrafabed
#115 1234624-115-limit-values-to-display.patch79.77 KBashrafabed
#105 1234624-105-limit-values-to-display.patch30.04 KBsavkaviktor16@gmail.com
#87 field_limiter.zip5.09 KBmarcvangend
#80 1234624-80-limit-values-to-display.patch29.7 KBmarcvangend
#76 1234624-76-limit-values-to-display.patch24.75 KBmarcvangend
#73 1234624-73-limit-values-to-display.patch11.7 KBmarcvangend
#71 1234624-71-limit-values-to-display.patch4.29 KBmarcvangend
#61 formatterbase-range3.patch3.55 KBAnonymous (not verified)
#59 formatterbase-range.patch3.54 KBAnonymous (not verified)
#56 formatterbase-range.patch2.36 KBAnonymous (not verified)
#53 drupal-1234624.patch60.17 KBSweetchuck
#43 1234624-43.patch8.02 KBswentel
#30 limit-display-fields1234624-30.patch4.03 KBandrewmacpherson
#28 limit-display-fields-1234624-28.patch3.91 KBandrewmacpherson
#27 limit-display-fields1234624-27.patch0 bytesandrewmacpherson
#25 limit-display-fields1234624-25.patch3.91 KBandrewmacpherson
#22 1234624-22.patch3.92 KBswentel
#20 1234624-summary.png48.17 KBswentel
#20 1234624-form.png54.95 KBswentel
#20 1234624-20.patch3.92 KBswentel
#4 1234624-manage-display.png27.22 KBswentel
#4 1234624-manage-display-hover.png21.6 KBswentel
#4 1234624.patch4.15 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

subs

swentel’s picture

Ok, first version of a patch, still needs work, but setting to needs review for feedback. I've attached some screenshots as well so people can have a first impression.

Things that need work:
- upgrade path: save all instance settings with limit ?
- tests: upcoming

Feedback:
- is the preprocess_field the right place or should we cut of items earlier ?

(second try - stupid double click)

pcambra’s picture

Great idea indeed, just a random thought, maybe this should belong to the formatting options

droplet’s picture

missing a way to view all images ?

swentel’s picture

@droplet: if you leave that field empty, it shows nothing.
@pcambra: possibly. Although at this point we can only form alter into that form because it's a module specific thing (unless I've missed a hook). We could introduce a new hook or another technique of course.

Also, I've been thinking about the placement of this. Maybe we could even introduce '#display-limit' as a property for any sort of element and let drupal_render() handle this in the end. Thoughts about that ?

pcambra’s picture

yched’s picture

Status: Active » Needs review
Issue tags: +Needs usability review

Patch is fine code-wise.
Obviously the main challenge here will be to find the right UI shape. Thus, added the 'UX review tag'. I know UX folks *love* a good brainstorm on the Field UI :-D.

We're talking about generic settings that are relevant independently of the field type and formatter, as opposed to the existing formatter settings.
For the record, and if that brings food for the discussion : other much awaited features, in addition to this "# of values to display" setting, include :
- custom markup aka "semantic fields", currently also provided by Display Suite D7 (@swentel, maybe you have a screenshot of the current UI somewhere ?)
- separator for multiple values if the markup for multiple values is inlined (comma, dash, ...)
Of interest (or not ?) for the UI considerations, all those features pertain to multi-values fields, much less to single-value fields (or non-fields)

Contrib D7 currently already clings those features (exceot maybe the 'separator' one ?) as it can on the core Field UI screen - so this is not a hypothetical case. As the above comments show, *2* different contrib modules currently provide the '# of values to display' feature.

swentel’s picture

re: that markup, here's the screenshot http://realize.be/files/ds-field-template_0.png

Status: Needs review » Needs work

The last submitted patch, 1234624.patch, failed testing.

Bojhan’s picture

Why can't this be a formatting setting of the thing "in general"? If its potentially more, it should clearly not fit in the model shown above.

kclarkson’s picture

It would also be nice to have an option to show random images.

For example you have a node that has 10 images attached, you could have the teaser show one random image from that node.

Right now the random function only works with the "sort" settings for the complete node. So in the teaser I could not have my events sorted by date while also showing the nodes with random images.

Any thoughts?

droplet’s picture

@swentel #7,

I meant if it have 10 images and set to show 2 only. How can I view other 8 images ? There should be a link "View All Images" ?

yched’s picture

@droplet : that's out of scope IMO. But this brings an interesting point in the hairy issue of "read more" links. #823380: Read More link is always visible on teaser.

johnv’s picture

Subscribe

andrewmacpherson’s picture

subscribing, got a keen interest in developing field formatter settings.

@14 + @15:
If a link is out of scope, then how about "and 8 more". It doesn't have to be a link, just an indication that the items being disoplayed are not all there is. Even if it's out of scope for core, I think it's a good use-case for a contrib module. Let's see if we can make it easy to hook into this.

See Field multiple limit and other modules using Field formatter settings API.

droplet’s picture

Ok. I missed the topic.." but you only want to show the first on a teaser view. "

however, I don't see the patch limited settings on teaser view only. If not teaser view exclusively, my comment #14 is important. I hate what drupal does now, some of features 50% in Core, 50% in contrib module.

If only 50% functionally, it needs to rethink again. It's only 30 sites using the contrib module that do same thing.

"Reported installs: 30 sites currently report using this module."

yoroy’s picture

Good feature. I think "Show 1 thumb in the teaser where the full node has 4 images" is a pretty common use case and desirable feature for core. I'm imagining this as an option for each view mode (display mode), where each display mode has the default set to 'show all' (like it is now). Then you see those ugly teasers and start digging into field ui settings…

Hmm. Maybe a core default for the teaser display of the Article content type to limit to 1 would be better then :)

But discoverability of settings for fields is a general problem to be solved on a more fundamental rework of field ui and this is a great little use case to take into account then.

droplet: does a core default to 'show all' on full node already answer your concern?

swentel’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
3.92 KB
54.95 KB
48.17 KB

New patch. With #945524: Field formatter settings hooks in core going in, I took a complete different approach where the field module is now the first module in core implementing these hooks which makes sense to me. What do others think of this approach ? Another approach could be that we create a new module inside the field module with field formatter settings that we identify as useful for core.

The patch also moves the settings into the cogwheel. The cogwheel now also shows up nicely when a formatter by default didn't have any settings, e.g. the tags, see attached screenshots. Also, there's a limit and offset now.

1234624-summary.png

1234624-form.png

Status: Needs review » Needs work

The last submitted patch, 1234624-20.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
FileSize
3.92 KB

Urg, stupid variable mix up

Bojhan’s picture

Loosk good to me, kinda sounds like we should have a consistent pattern for this with views going into core, but meh @ consistency. Lets do this.

Status: Needs review » Needs work

The last submitted patch, 1234624-22.patch, failed testing.

andrewmacpherson’s picture

Improving the summary message when displaying all values.
Changed "display_offset all values" to read "Show all values".

andrewmacpherson’s picture

Status: Needs work » Needs review
andrewmacpherson’s picture

Needs a full stop as well.

andrewmacpherson’s picture

Sorry. uploaded an empty file somehow.

Status: Needs review » Needs work

The last submitted patch, limit-display-fields-1234624-28.patch, failed testing.

andrewmacpherson’s picture

I found a couple of problems:

  1. If the limit was not specified, but an offset was specified, NO items would display (e.g. show all values, skip first value). I have fixed this by carrying out two separate array slices. Skip the offset first, then limit what's left.
  2. If the offset is greater than the number of values actually stored, than no items display (expected), but the field label will still be shown (unexpected).
pcambra’s picture

Status: Needs work » Needs review
swentel’s picture

I'm wondering if we can add an element_validate to this form so that we can cleanup during submit so we can keep having only one array_slice.

Thanks for working on this btw!

The failing tests is because now the two additional keys are added. The main question still might be whether this belongs in field module or in a separate module or not. If we choose for the latter, the field module stays clean and we can possibly add more features to the 'field formatter settings' module. But for that, we need more ideas. Simply just having 'multiple field limit' as a submodule is an option maybe too.

Status: Needs review » Needs work

The last submitted patch, limit-display-fields1234624-30.patch, failed testing.

xjm’s picture

yoroy’s picture

Bump! One of those 'normal' but very jummy feature requests :)

#32 has the to-do's for pushing this further along…

klonos’s picture

...another friendly bump here in order to propose to include a randomizer feature (show x number of values - randomly).

yched’s picture

@yoroy: does it mean you validate the UI ? :-)

swentel’s picture

@yched so do you also validate the approach then ? :) (it's a controversial one I think)

Also, re-looking at the patch again, it doesn't make sense to have a hook in field and one in field_ui, also, the one in field_ui is always going to run, so this one needs work anyway.

yched’s picture

Actually, no, I don't validate the approach ;-). This shouldn't be done by squatting the 'settings' section, which is the domain of each individual formatter type.

But if the UX team is fine with the proposed UI, then I'm fine with getting this in as is, and clearing after Dec 1st.
Because, yes, feature-wise, it would be a very welcome addition.

yched’s picture

Sorry, I realize #39 is unclear - what I mean is :
I'm not fine with the code, those properties shouldn't be stored in the 'settings' section of $instance['display'][$view_mode], but directly at the top level as system-supported, formatter-agnostic properties.

But I'm fine with the UI, and I don't think we can have a better one until the modals patch lands and we can refactor Field UI around modals.

So if the UX team is fine with the UI, +1 on getting this in as is and cleaning the code past freeze.

swentel’s picture

It was perfectly clear, don't worry :) I wasn't actually fond of the approach either. I'll look at the failing tests + adding one for the functionality.

Bojhan’s picture

Issue tags: -Needs usability review

This was already signed off.

swentel’s picture

Status: Needs work » Needs review
FileSize
8.02 KB

Should fix the tests + added tests for limit and offset functionality.

yoroy’s picture

Yup, the UI is what we can do at this stage. It's green, too!

RTBC?

attiks’s picture

This looks good, only found some minor comment issues.

+++ b/core/modules/field/lib/Drupal/field/Tests/DisplayApiTest.phpundefined
@@ -139,6 +139,39 @@ function testFieldViewField() {
+    // Test on limit setting of fields with cardinality not set to 1.

The comment is confusing to me, but might just be me.

+++ b/core/modules/field/lib/Drupal/field/Tests/DisplayApiTest.phpundefined
@@ -139,6 +139,39 @@ function testFieldViewField() {
+    // Limit

THis is a bit short and is missing a trailing period.

andypost’s picture

#43: 1234624-43.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1234624-43.patch, failed testing.

andypost’s picture

Is this possible for D8? patch was nearly RTBC but needs re-roll

andrewmacpherson’s picture

Status: Needs work » Needs review

#43: 1234624-43.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1234624-43.patch, failed testing.

andrewmacpherson’s picture

Issue tags: +SprintWeekend2013

I tried re-rolling this patch over weekend. The reason it no longer applies is that some tests were removed from core. However I couldn't figure out where they should go now, or even if they are still necessary.

As it happens, commits were being made to core all weekend, so I never succeeded in making a patch that applied cleanly.

On the positive side, I managed to port field_multiple_limit.module to 8.x in contrib, and rolled functioning alpha release!

I would still prefer it see it in core though.

johnv’s picture

Testing D8 patches is not (yet) my cup of tea. Someone created a nice D7-module: http://drupal.org/project/vase
A new issue #1968626: There's a patch for that! points to this core issue.

Sweetchuck’s picture

FileSize
60.17 KB

Here is my concept. (very-very early stage)

yched’s picture

Version: 8.x-dev » 9.x-dev

@Sweetchuck: not workable that way. 'settings' is not the place to support generic features supported across all formatters - see #39 / #40.

Also, I'm afraid at this point it's too late for feature requests in the D8 cycle :-/

BrightBold’s picture

Issue summary: View changes

Does this now have the chance to make it into 8.1+ now that the release cycle has changed? It's a good feature.

Anonymous’s picture

Status: Needs work » Needs review
FileSize
2.36 KB
Anonymous’s picture

Version: 9.x-dev » 8.x-dev

d8 to trigger tests.

Anonymous’s picture

Version: 8.x-dev » 9.x-dev

ehm

Anonymous’s picture

FileSize
3.54 KB

fixed file/image fields

andypost’s picture

no reason for 9.x this could be done in 8.1

Anonymous’s picture

FileSize
3.55 KB

Class replaced with interface

Status: Needs review » Needs work

The last submitted patch, 61: formatterbase-range3.patch, failed testing.

dawehner’s picture

Nice!

  1. +++ b/core/lib/Drupal/Core/Field/FormatterBase.php
    @@ -104,23 +104,88 @@
       /**
    +   * Returns the cardinality setting of the field instance.
    +   */
    +  public function getCardinality() {
    

    Isn't that better as a protected method?

  2. +++ b/core/lib/Drupal/Core/Field/FormatterBase.php
    @@ -104,23 +104,88 @@
    +    if ($this->getCardinality() == 0) {
    ...
    +    if ($this->getCardinality() == 0 || (empty($limit) && empty($offset))) {
    ...
    +      if ($this->getCardinality() != 0) {
    

    do we have a magic constant we could use here?

Anonymous’s picture

As far as I know only unlimited cardinality value has a constant.

mgifford’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

marcvangend’s picture

This issue has been dead a long time - is there a reason why we cannot or should not bring it back to life?

rootwork’s picture

I've been following this issue forever, and I think it would be really useful. @marcvangend if you want to reroll the patch and address #63 I think that would be excellent!

marcvangend’s picture

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

OK, I'll see what I can do. It will have to be re-rolled against 8.2.x if I understand correctly.

rootwork’s picture

It would be a new feature, so yes you're right. Thanks for working on it!

marcvangend’s picture

FileSize
4.29 KB

Here's a re-roll of #61. I changed three things:

1) I made getCardinality() a protected method as per #63.

2) In FormatterBase::settingsForm(), I set $element['limit']['#min'] to 0. It used to be 1, but then you cannot save the form anymore when you want to go back to 0.

3) In FormatterBase::prepareView, the code now proceeds to limit the displayed values if either $offset or $limit is not empty. This (in combination with change 2 above) allows you have an offset without setting a limit, so all values beyond the offset are displayed.

---

For further improvement, I'm curious if we can improve the UX a little. The fields for offset and limit do what they're supposed to, but you cannot really see that they belong together. Maybe we can make it more human readable and compact by turning it into a sentence like this:

DISPLAY ITEMS
Display [____] items, while skipping the first [____].

On the other hand this is not a design pattern that is common in Drupal core. I would love to know what UI/UX people think.

marcvangend’s picture

Status: Needs work » Needs review
marcvangend’s picture

Updated patch, to make sure that all classes that extend FormatterBase::settingsSummary() call the parent method first, before adding their own summary data.

By the way I haven't looked at tests yet, let's see what the testbots think first.

The last submitted patch, 71: 1234624-71-limit-values-to-display.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 73: 1234624-73-limit-values-to-display.patch, failed testing.

marcvangend’s picture

Status: Needs work » Needs review
FileSize
24.75 KB

OK, bear with me, because I'm still learning about config schema definitions. This patch is a bit of a gamble, hoping that it will fix (some of) the test failures. If there are better ways to do the same, please let me know.

Status: Needs review » Needs work

The last submitted patch, 76: 1234624-76-limit-values-to-display.patch, failed testing.

marcvangend’s picture

Yay, down from 829 to 3 fails. To be continued.

rootwork’s picture

Now that's progress! :)

marcvangend’s picture

Status: Needs work » Needs review
FileSize
29.7 KB

See if this comes back green.

dawehner’s picture

Note: It is not required actually to change the base class. There are alternative mechanisms to change output of a field. For example https://github.com/drupal-media/field_formatter/pull/5 provides a wrapper for formatters which does the same.
But yeah this is just some suggestion what could be done instead.

marcvangend’s picture

Thanks for the code example @dawehner. I'm trying to figure out exactly how it would work for a site builder; does it mean that the "limiting formatter" would become a separate formatter that can be selected like other formatters? Or would it automatically affect all available formatters, just like the patch in #80does?

dawehner’s picture

@marcvangend
Yeah I certainly don't say that this patch is not right or anything like that. I just wanted to provide an alternative way.

What this patch would do, is to provide a formatter, which allows you to configure a) limit b) offset and c) the wrapped formatter + formatter settings. So its a bit like formatterception.

marcvangend’s picture

@dawehner Aha, now I see... Not all of the magic is in that PR, part of it is already in the FieldWrapperBase class of the Field Formatter module.

What I like about the wrapper idea, is the separation. After all the limiter is not really a formatter; it is more like a generic preprocessor for any formatter. The wrapper concept allows formatters and "formatter preprocessors" to be mixed and matched. Personally I'm not a huge fan of modules that bundle all kinds of useful helpers in one package (Ctools being the no.1 example), but I guess that's just a matter of personal taste.

Anyway, at this point I would love to get some feedback of Field subsystem maintainers, and ask them what they think of the direction the patch is going, the feasibility of getting this into core, and alternative solutions like the field formatter wrapper.

Anonymous’s picture

I have simple custom plugin type that also handles this - https://gist.github.com/ivanjaros/d079714f12161d239550f6bb506b35df

and it is being invoked like so:


/**
 * Implements hook_entity_display_build_alter().
 */
function hook_entity_display_build_alter(&$build, $context) {
  /** @var \Drupal\store\Plugin\store\FieldFormatterEnhancer\FieldFormatterEnhancerPluginManager $manager */
  $manager = \Drupal::service('plugin.manager.field_formatter_enhancer');

  /** @var \Drupal\Core\Entity\Display\EntityViewDisplayInterface $display */
  $display = $context['display'];

  foreach ($display->getComponents() AS $field => $field_information) {
    // Make sure the field if displayed and not empty.
    if (isset($build[$field]) && isset($build[$field]['#items'])) {
      $build[$field] = $manager->viewField(
        $build[$field],
        $build[$field]['#items']->getFieldDefinition(),
        $display->getRenderer($field)
      );
    }
  }
}

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.

marcvangend’s picture

FileSize
5.09 KB

I created a separate module, based on the code by @dawehner and dependent on the field_formatter module. Hopefully it can help get this issue moving again.

The module works with all core and contrib field types I tested against (from simple text fields to more complex things like paragraphs).

That said, I'm not too happy with some implementation details, especially in FieldLimiter::viewElements(). Currently the limiter formatter simply lets the actual formatter render all values, and then takes out the elements it wants to keep. That's not ideal from a performance point of view. Also, it makes it impossible for the actual formatter to take the number of field values into account. I would rather alter the FieldItemListInterface object and limit the field values before they are passed to the actual formatter, but I don't know if/how that's possible.

marcvangend’s picture

Wait, I think I figured out how to limit the values in the FieldItemListInterface:

    $field_values = $items->getValue();

    $offset = $this->getSetting('offset');
    // To show all elements, expressed by the setting '0', array_slice needs
    // NULL as 3rd argument.
    $limit = $this->getSetting('limit') == 0 ? NULL : $this->getSetting('limit');

    // Let array_slice limit the field values to the ones we want to keep.
    $limited_values = array_slice($field_values, $offset, $limit);
    $items->setValue($limited_values);

I'll try to upload a new version later.

marcvangend’s picture

All right, I created an improved version and created a sandbox project for it: https://www.drupal.org/sandbox/marcvangend/2843702. It's not necessarily my goal to turn it into a full module, I'm open to merging the code with Field Formatter, Field Multiple Limit, or even core. Feedback appreciated.

rootwork’s picture

@marcvangend Message andrewmacpherson, he's the maintainer of Field Multiple Limit for D8. I suspect he would welcome collaborating and/or merging, as the current D8 port has been stuck in a broken state for a little while now.

rootwork’s picture

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

We certainly need some test coverage for the new functionality here.

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.

fgm’s picture

Related issues: +#2864687: Drupal 8 version

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.

AdamPS’s picture

@marcvangend Nice! The wrapper formatters seem like a great idea and I agree they could give a preferable user experience than the earlier patch of #80.

  • "Field Limiter" is an excellent example.
  • Field formatter offers "Link field formatter" that wraps the target in a link.
  • The community will likely think of many other examples. Perhaps a formatter that wraps the output in a div with the specified CSS class?

The use case for any one of the possibilities is probably small, so to make the inputs appear for all fields and all users always could be seen as too much. I like that the wrapper formatter allows users to add in the fields when they want them.

tim.plunkett’s picture

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.

larowlan’s picture

+++ b/core/lib/Drupal/Core/Field/FormatterBase.php
@@ -109,23 +109,88 @@ public function view(FieldItemListInterface $items, $langcode = NULL) {
+    }
+    return 0;

what scenarios are there where this would happen?

jds1’s picture

I was able to work around this using Paragraphs. I had a multivalue link field where I only wanted one value to display at a time, up to 7. I created a "Link" paragraph type with a single "Link" field, then added a Paragraphs reference field with a limit of 7. One link shows by default and then there's an "Add Link" button that lets me add more as needed (up to the limit). The patch didn't apply for me on 8.5.x and while I am sure the field_limiter module is rad, I would rather use a contrib solution and wait until we get something in core.

Hope this helps someone!

andypost’s picture

  1. +++ b/core/lib/Drupal/Core/Field/FormatterBase.php
    @@ -109,23 +109,88 @@ public function view(FieldItemListInterface $items, $langcode = NULL) {
    +    if ($this->getCardinality() == 1 || (empty($limit) && empty($offset))) {
    ...
    +    $summary[] = $this->t('Display %limit items, skip %offset items.', array(
    ...
    +        if (!empty($offset) || !empty($limit)) {
    

    offset could be empty when limit is not

    so empty($offset) should be removed and message build needs rework

  2. +++ b/core/modules/image/src/Plugin/Field/FieldFormatter/ImageFormatter.php
    @@ -125,14 +125,14 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    -    return $element;
    +    return parent::settingsForm($form, $form_state) + $element;
    ...
    -    $summary = array();
    +    $summary = parent::settingsSummary();
    
    +++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
    @@ -138,7 +138,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    -    $summary = array();
    +    $summary = parent::settingsSummary();
    

    could be decoupled to separate issue to fix

AdamPS’s picture

As I understand it the patch in #80 and zip in #87 are out of date based on the subsequent comments, hence I have hidden them.

The proposed next steps are outlined in #89. The newer solution is much more powerful and flexible: site builders can selectively add limits to precisely the fields that need them, and furthermore add other wrappers to other fields. It doesn't force all sites and all fields to have limit settings that are only used in <1% of cases.

If we want this in core, it looks like it would be a matter of adding:

Possibly also we could also add some other wrappers:

  • Field formatter "Link field formatter" which wraps the target in a link.
  • "CSS class formatter" that wraps the output in a div with the specified CSS class.

On the other hand, as per #89 it doesn't have to be in core and would work well in contrib. It now needs people to make proposals as to where would be the best place for the code to live.

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.

andypost’s picture

Issue tags: +Needs reroll
savkaviktor16@gmail.com’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
30.04 KB

Re-rolled (without making any suggested changes)

Status: Needs review » Needs work

The last submitted patch, 105: 1234624-105-limit-values-to-display.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

MrPaulDriver’s picture

I often require such utility and would be fine with this being in contrib.

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.

goodDenis’s picture

Thanks, #105 works for me.

marcvangend’s picture

Long overdue, Field Limiter is now a full module. All feedback is still very welcome, since it is impossible to test the module with the infinite possible combinations of fields and formatters.

AdamPS’s picture

Assigned: swentel » Unassigned
Issue summary: View changes
Status: Needs work » Active
Issue tags: -SprintWeekend2013, -Needs subsystem maintainer review, -Needs tests, -Blocks-Layouts

Great thanks @marcvangend

Shall we close this issue now? Or I guess we can leave it open to track the idea to move the "field wrapper" concept into core?

It looks like the tags and assigned field are now out of date - sorry if that's wrong please feel free to put them back.

tim.plunkett’s picture

Issue tags: +Blocks-Layouts

Still tracking this.

mpp’s picture

Issue summary: View changes
ashrafabed’s picture

Responding to:
"On the other hand, as per #89 it doesn't have to be in core and would work well in contrib. It now needs people to make proposals as to where would be the best place for the code to live."
and
"Shall we close this issue now? Or I guess we can leave it open to track the idea to move the "field wrapper" concept into core?"

I personally feel this should be implemented in core:
1) The views module already has an implementation somewhat similar to this
2) Using Layout Builder we can now place individual fields on a page, repeatedly if we like. I would like to place the first value of the field once, then the subsequent values of the same field separately. This patch enables that sort of ability. ( https://www.drupal.org/project/drupal/issues/3029830 )

Wanted to make sure people in this issue (aside from tim.plunkett) were aware of the use case. If I continue working on use case #2 in LB without this patch, I will end up re-implementing this functionality in a hacky way. So +1 for this in core.

ashrafabed’s picture

Rerolled the patch and addressed comments from #99 and #101.

tim.plunkett’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 115: 1234624-115-limit-values-to-display.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

AdamPS’s picture

I am surprised that a patch for this approach posted yet again - the catch is that it adds new settings everywhere that most people won't need.

As per #102, #110 and many earlier comments I feel that the approach in the patch has been overtaken by a much better approach implemented in the contrib module FieldLimiter. The "field wrapper" concept would be a great addition to core that has many possible use cases. This way, the new settings are only present for the people that want them.

Please can anyone people still posting on the older approach take a look at this new alternative? If you aren't convinced then please explain why rather than just reposted the older patch again.

From #102:

The newer solution is much more powerful and flexible: site builders can selectively add limits to precisely the fields that need them, and furthermore add other wrappers to other fields. It doesn't force all sites and all fields to have limit settings that are only used in <1% of cases.

If we want this in core, it looks like it would be a matter of adding the FieldWrapperBase from Field Formatter as well as the Field limiter itself.

ashrafabed’s picture

Thanks for the quick response. When you said "The newer solution" I thought you meant the one in the newest patch - only worked on this during commute so didn't have much time to read the comments more closely.

I'll think about the two approaches and circle back.

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.

andypost’s picture

As I got the question in storage?

- formatter settings - will affect every configured field formatter in every display
- third party settings - could be optional

But formatter base class could exclude settings for limit if it's not enabled.

At least the issue needs work for tests and other approach patch to evaluate performance - as it affects every formatter

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.

andypost’s picture

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

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.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.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.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, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.