Improve layout for Multiple Field Settings in Views

Thanks for the issue and patch.

1) Patch applies cleanly to 9.1.x.

2) Issue without patch was confirmed. See screenshots.

3) Improvement with patch was confirmed. See screenshots.

4) Code change is simple though I'm not sure that is the correct fix.

5) Now that Start from last values is separated on its own line, it might need a period at the end, but I'm not sure so that could be added during commit.

6) New automated tests aren't needed.

7) Dropped to Minor. Leaving as Bug Report but maybe this is better as a Task.

8) Kicking off tests.

Looking at the change more carefully, it seems ok to me. It's just moving the ending </div> from the delta_first_last form field to the delta_offset in order to force the break earlier.

Marking RTBC based on the last two comments and that tests are green.

Yeah, let's make this a task.

The current situation is very messy indeed, so the 'after' does look better. One thing though, the current situation does make it clear (well ok, clearer at least) that these 3 settings are connected, which does get a little lost in the 'after' situation.

Thanks. Yeah, the grouping is lost but I also think it's easier to grok with the change.

It might be worthwhile reviewing this on one of the UX calls

It appears that this is retesting the 8.4.x patch. So, I am re-uploading the patch so it tests with 9.2.x

I think if we're changing this we need to do a couple of things to make it easier to grok.

1. We need to move
   

       // Make the string translatable by keeping it as a whole rather than
       // translating prefix and suffix separately.
       list($prefix, $suffix) = explode('@count', $this->t('Display @count value(s)'));
   

   to just above $form['delta_limit'] = [ so it is closer to where it is used (and then overridden).

2. We fix a bug introduced by #1261528: Add "First and last only" option to Multiple field settings. We need to remove '#suffix' => $suffix, from $form['delta_reversed'] = [ it has no business being there.
3. It's tempting to take this opportunity to add

       $form['multiple_field_settings_inline'] = [
         '#type' => 'container',
         '#attributes' => ['class' => ['container-inline']],
         '#fieldset' => 'multiple_field_settings',
       ];
   

   And then move delta_limit and delta_offset to $form['multiple_field_settings_inline']['delta_limit'] and $form['multiple_field_settings_inline']['delta_offset'] respectively. And then change their '#fieldset' => 'multiple_field_settings' to '#parents' => ['options', 'delta_limit'] and '#parents' => ['options', 'delta_offset'] respectively. This would rid of us of have to work out the difference of #prefix and #field_prefix and tidy this up. I think using #parenets is okay here - we do the same in \Drupal\views\Plugin\views\HandlerBase::buildOptionsForm() for the admin label.

Addressed points mentioned in #17.

Rerolled #18

changed position of the following code $form['multiple_field_settings_inline'] to just above of the following line $form['multiple_field_settings_inline']['delta_limit']

Applied patch in #27 successfully. Moved the "Reversed" and "First and last" checkboxes down and addressed points mentioned in #17. Adding screenshots for reference

Hi applied patch#24 on Drupal 10.0.x. patch worked perfectly fine.

Thank you @gaurav-mathur but the screenshots were added in #28 so this was duplicated effort.

Reviewing the code

list($prefix, $suffix)
1. Think we shouldn't be using list instead array

Tagging for novice.

Tried to address #31.