New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added new setting field to configure multiple values #12807
Conversation
Re name: I think the 2 value version could be named |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. I would approach the architecture differently, though. Instead of a one multipair control that is a list of pairs, there would be two UI controls, one being a list
control that would manage the '-'/'+' buttons and would take a child control, which could be pair
or tuple
(or any other control), which would list specific inputs next to each other in groups.
if ($scope.field1 && $scope.field2) { | ||
if (!table[$scope.field1.key] && !table[$scope.field2.key]) { | ||
hasAll = false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If table
has field1 but not field2, hasAll == true
? If yes, hasAll
might be named incorrectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cheers, renamed it
@diosmosis renamed it to |
@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 👍 |
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 As a way to visualize this, imagine a tree of components like this:
|
Tested it locally, noticed some odd UX behavior:
Entering text suddenly adds a new row: 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).
(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.)
|
I just tried it out and it is working great. Only "error" is that the hover of the delete icon shows Should I load it manually? |
We should simply add it here: https://github.com/matomo-org/matomo/blob/3.x-dev/plugins/CoreHome/CoreHome.php#L387 |
* Added new setting field to configure multiple values * remove not needed comment * better spacing between rows * make fields wider * fix ui test * rename multipair to multituple * rename variable * fix it was not possible to persist nested arrays * only catch error when using old version * catch exception * better implementation * require higher version * trying to fix json_encoded columm missing in db during updater * silence errors * hard code the query * fix user login cannot be null * fail if query fails
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:
select + text:
and it is even possible to configure only one field like this:
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
: