[GitHub] [brooklyn-ui] tbouron commented on issue #129: UI for defining parameters

[GitBox <gi...@apache.org>]

tbouron commented on issue #129: UI for defining parameters
URL: https://github.com/apache/brooklyn-ui/pull/129#issuecomment-478991360
 
 
   >yeah, i buy that. i don't see an easy way to do it with validation for the case where duplicate keys are set in yaml. (we already did it if you tried to supply duplicate keys in the ui so i think from yaml is the only way.) what i've done instead is that on model load if we find a duplicate we append a number to it, so brooklyn.parameters: [ { name: y, type: string }, { name: y, type: string }, { name: y, type: string } ] becomes y y2 y3, with warnings logged to the console. it's awkward and surprising but at least doesn't break the ui. we could have yaml model validation errors as well for this though the problem would still manifest in the graphical composer if invalid yaml was passed in so we need a way to handle it here. the other option would be to refuse to show parameters.
   
   Sounds good for now 👍 
   
   > if it's valid JSON it works, but it must be e.g. ["required"] -- -have changed message to note it needs the brackets and double quotes. could improve by supporting yaml parsing or a dedicated editor but going for minimal working capability here. also agree clearing on unfocus is awkward but of course there's a lot of work to store invalid models if we did something else. we could maybe forbid the unfocus.
   
   Aaaah it requires JSON! Great for the error message, **it would be also great to specify this in the information popup for the constraints @ahgittin.**
   
   > doesn't moot mean "no longer relevant or not worth discussion"? i think you mean it's a step backwards. personally i thought having two was awkward and one is fairly self-explanatory. happy to see further evolution and testing but agree it's such a minor point as to be moot :).
   
   Probably my flawed French translation: `moot` means `subject to debate, arguable` to me. 
   
   > i removed checkboxes so in total we have a real estate savings. i think icons make it look more polished, but agree the x is confusing. have changed it to a stop-circle (bit white square in black circle). again happy to see further evolution but i'd like to keep the icons. if you feel strongly let's get a tie-break opinion!
   
   Great!
   
   Happy to merge once we have the explanation for the JSON constraints @ahgittin !

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services