Skip to content
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

Jump to bottom

Do not show default value when default value is an array #12824

Merged
merged 3 commits into from Aug 2, 2018
Merged

Do not show default value when default value is an array #12824

merged 3 commits into from Aug 2, 2018

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented May 5, 2018 •
edited

Otherwise it shows a json encoded array as default value.
image

Especially in combination with the new form field multi tuple added in #12807

@tsteur tsteur added the Needs Review PRs that need a code review label May 5, 2018
@sgiehl
Copy link
Member

sgiehl commented May 15, 2018

Maybe we should adjust the Demo UI page to include default values. If I have seen that correctly there are currently none...

@mattab mattab added Needs Review PRs that need a code review and removed Needs Review PRs that need a code review labels Jun 26, 2018
@mattab mattab added this to the 3.6.0 milestone Jun 26, 2018
@sgiehl sgiehl force-pushed the nodefaultvaluearraypretty branch from b917c59 to ad1b0b6 Compare July 7, 2018 12:56
sgiehl
sgiehl requested changes Jul 16, 2018
View reviewed changes
plugins/CorePluginsAdmin/angularjs/form-field/form-field.directive.js Outdated
@@ -281,6 +281,10 @@
function formatPrettyDefaultValue(defaultValue, availableOptions) {

if (!angular.isArray(availableOptions)) {
if (angular.isArray(defaultValue)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I see that correctly, the defaultValue is always a string here, as it isn't converted at any point.
Guess it would need something like this when getting the defaultValue:

if (field.uiControl === 'multituple' && angular.isString(defaultValue) && defaultValue) {
   defaultValue = JSON.parse(defaultValue);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgiehl did that work for you? It still serialized it to json afterwards for me even though it did JSON.parse it (tested with debugger)

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't test that before. but really doesn't work as expected. But can't figure out why right now...

@sgiehl sgiehl force-pushed the nodefaultvaluearraypretty branch from ad1b0b6 to 097229e Compare July 16, 2018 12:59
@tsteur
Copy link
Member Author

tsteur commented Jul 24, 2018

@sgiehl is it ok to just merge for now and not show default value when default is an array/object which may be complex to show anyway?

@sgiehl
Copy link
Member

sgiehl commented Jul 25, 2018

For me the changes simply don't work as expected. Here's an example of an multituple field that still shows the default:

                <div piwik-field uicontrol="multituple" name="multitupletextvalue"
                     title="Multiple values with values"
                     default='[{"index": "test", "value":"myfoo"},{"index": "test 2", "value":"myfoo 2"}]'
                     value='[{"index": "test", "value":"myfoo"}]'
                     inline-help="Multi Tuple again."
                     ui-control-attributes='{"field1":{"key":"index","title":"Index","uiControl":"text","availableValues":null},"field2":{"key":"value","title":"Value","uiControl":"text","availableValues":null}}'>
                </div>

@mattab mattab modified the milestones: 3.6.0, 3.7.0 Aug 1, 2018
@tsteur
Copy link
Member Author

tsteur commented Aug 2, 2018

@sgiehl can you check again?

@sgiehl sgiehl merged commit 1fb40ac into 3.x-dev Aug 2, 2018
@sgiehl sgiehl deleted the nodefaultvaluearraypretty branch August 2, 2018 15:24
@sgiehl sgiehl modified the milestones: 3.7.0, 3.6.0 Aug 2, 2018
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
@tsteur @InfinityVoid
Do not show default value when default value is an array (matomo-org#…
cc3639a 
…12824)

* Do not show default value when default value is an array

* make sure a pretty value is actually set

* do not show default value for multituple
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees

@tsteur tsteur

Labels
Needs Review PRs that need a code review
Projects
None yet
Milestone
3.6.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tsteur @sgiehl @mattab