@tsteur opened this Pull Request on May 2nd 2018 Member

We had this developed a couple of times in various plugins and lots of duplicated code. As I now needed it as part of the settings API, I finally made it available through the API. I named it "multi pair" as in you can receive multiple values each consisting of a pair / two values. Couldn't think of a different name. With this you can basically receive a value like [{mykey1: myvalue1, mykey2: myvalue2}, {...}, ...]

You can combine various fields like text + text:
image

select + text:
image

and it is even possible to configure only one field like this:
image

Later we might change it to accept also 3 or 4 or 5 ... fields in a generic way although the main problem is positioning them nicely next to each other etc.

It is a basic, quite flexible solution that we can extend later with more features if needed.

I had to make this work with the settings/fieldConfig API so kind of misused uiControlAttributes:

            $this->makeSetting('fooBar', array(), FieldConfig::TYPE_ARRAY, function (FieldConfig $field) {
                $field->title = 'Foo Bar';
                $field->description = 'Optionally set one or multiple custom dimensions in scope visit.';
                $field->uiControl = FieldConfig::UI_CONTROL_MULTI_PAIR;
                $field1 = new FieldConfig\MultiPair('Index', 'index', FieldConfig::UI_CONTROL_TEXT);
                $field2 = new FieldConfig\MultiPair('Value', 'value', FieldConfig::UI_CONTROL_TEXT);
                $field->uiControlAttributes['field1'] = $field1->toArray();
                $field->uiControlAttributes['field2'] = $field2->toArray();
            }),
@diosmosis commented on May 2nd 2018 Member

Re name: I think the 2 value version could be named pair since in the code you're basically creating a n array field of a pair of values. If you made the number of fields generic, you could call it a tuple.

@tsteur commented on May 2nd 2018 Member

@diosmosis renamed it to multituple

@tsteur commented on May 2nd 2018 Member

@diosmosis I don't 100% understand what you mean re changing the structure but could change it once we add more fields and make it more generic if it is of any help 👍

@diosmosis commented on May 2nd 2018 Member

First off, let me say the comment isn't something I think is a requirement and I don't even know if it's possible server side. The idea is basically to separate the responsibilities of multipair out. Instead of multipair, which manages a list (add / remove), and manages pairs/tuples (set/unset field1, set/unset field 2, ...), there would be a list component that would add/remove generic fields in an array, and a pair/tuple component that would manage setting fields on an object.

As a way to visualize this, imagine a tree of components like this:

- List
    - Tuple (Index text input, Value text input)
        - Text (Index)
        - Text (Value)
    (next to tuple: Delete Icon)
    - Tuple (Index, Value)
        - Text (Index)
        - Text (Value)
    (next to tuple: Delete Icon)
    ...
(after list: Add Icon)
@diosmosis commented on May 2nd 2018 Member

Tested it locally, noticed some odd UX behavior:

1) In the initial state, there's no remove/add icon:

image

Entering text suddenly adds a new row:

image

Since there's no visual cue that that would happen, I found it very surprising. It made me think I had to fill out the extra row (ie, it's not obvious it's just for adding a new item, and not a required item).

2) Not exactly sure how, but I was able to get to state where entering text into the bottom left row didn't create a new row:

image

(seems to be when one of the previous rows is empty. doubt it would happen a lot, but I think it would be an odd surprise.)

3) Then I tried entering something into the top row and it suddenly added a new row:

image

4) Saving w/ two blank rows and reloading gave me an angular error I can't recover from w/o running a SQL query:

image

@Findus23 commented on May 7th 2018 Member

I just tried it out and it is working great.

Only "error" is that the hover of the delete icon shows The string General_Remove was not loaded in javascript. Make sure it is added in the Translate.getClientSideTranslationKeys hook.

Should I load it manually?

@sgiehl commented on May 7th 2018 Member
This Pull Request was closed on May 3rd 2018
Powered by GitHub Issue Mirror