Use textarea for charm config entries.

Use textarea for charm config entries.

If a charm config default value has multiple lines, then use a textarea box
for the entry in the charm config panel.

The textarea has some known styling issues when a scrollbar is automatically
shown.  Given the infrequency of this styling being shown it has been OK'd for
landing with these visual deficiencies.

https://code.launchpad.net/~bac/juju-gui/1170468/+merge/160435

(do not edit description out of merge proposal)

[bac, 2013-04-23 16:57:19]

Please take a look.

[gary.poster, 2013-04-23 18:00:56]

LGTM, thank you!

Gary

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode622
app/views/utils.js:622: // For simplicity, numLines will be 0 if not multi-line
or 2 or more
0 or 1 if not multi-line, right?

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode623
app/views/utils.js:623: // if there are multiple lines.
Could you file a juju core bug about this (requesting an explicit multi-line
type, or something like that, explaining the use case), or make me do it (happy
to), and then reference the bug here?

https://codereview.appspot.com/8922043/diff/1/test/test_utils.js
File test/test_utils.js (right):

https://codereview.appspot.com/8922043/diff/1/test/test_utils.js#newcode496
test/test_utils.js:496: describe('utils.extractServiceSettings', function() {
Good tests, thank you!

[benjamin.saller, 2013-04-23 18:57:03]

LGTM 

The comment thing is a trival but should be corrected.

Thanks!

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode622
app/views/utils.js:622: // For simplicity, numLines will be 0 if not multi-line
or 2 or more
On 2013/04/23 18:00:56, gary.poster wrote:
> 0 or 1 if not multi-line, right?

Yes, this comment appears to be wrong.
"".split("\n").length === 1

but the test below (>1) is valid.

https://codereview.appspot.com/8922043/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/8922043/diff/1/lib/views/stylesheet.less#newco...
lib/views/stylesheet.less:1110: height: 15px;
Shouldn't this be a function of 'em' rather than 'px'. I don't care if we change
it now, but in general scaling fonts with em makes more sense. If we did want px
and because LESS can do math it might make sense to scale it off font-size, but
1.2em or somesuch makes more sense to me.

[bac, 2013-04-23 19:41:05]

*** Submitted:

Use textarea for charm config entries.

If a charm config default value has multiple lines, then use a textarea box
for the entry in the charm config panel.

The textarea has some known styling issues when a scrollbar is automatically
shown.  Given the infrequency of this styling being shown it has been OK'd for
landing with these visual deficiencies.

R=gary.poster, benjamin.saller
CC=
https://codereview.appspot.com/8922043

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode622
app/views/utils.js:622: // For simplicity, numLines will be 0 if not multi-line
or 2 or more
On 2013/04/23 18:57:04, benjamin.saller wrote:
> On 2013/04/23 18:00:56, gary.poster wrote:
> > 0 or 1 if not multi-line, right?
> 
> Yes, this comment appears to be wrong.
> "".split("\n").length === 1
> 
> but the test below (>1) is valid.

Done.

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode622
app/views/utils.js:622: // For simplicity, numLines will be 0 if not multi-line
or 2 or more
Originally I tried to skate by without isMultiLine and just rely on rows being 0
or >=2.  That was too hacky so I backed it out but forgot to fix the comment.

The comment has been removed entirely as it is no longer needed.

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode622
app/views/utils.js:622: // For simplicity, numLines will be 0 if not multi-line
or 2 or more
Originally I tried to skate by without isMultiLine and just rely on rows being 0
or >=2.  That was too hacky so I backed it out but forgot to fix the comment.

The comment has been removed entirely as it is no longer needed.

https://codereview.appspot.com/8922043/diff/1/app/views/utils.js#newcode623
app/views/utils.js:623: // if there are multiple lines.
On 2013/04/23 18:00:56, gary.poster wrote:
> Could you file a juju core bug about this (requesting an explicit multi-line
> type, or something like that, explaining the use case), or make me do it
(happy
> to), and then reference the bug here?

Done.

https://codereview.appspot.com/8922043/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/8922043/diff/1/lib/views/stylesheet.less#newco...
lib/views/stylesheet.less:1110: height: 15px;
Maybe but I'm hesitant to get into it in this branch.  You'll note I just moved
the existing height specification for an input[type=text] box to this location
so that a single string input has this specified height but it is unset for the
textarea and the size is computed by the 'rows' parameter.