launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/resizing_fix_911853 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #911853 in Launchpad itself: "auto resizing textarea on bug submit  jumps in FF8"
  https://bugs.launchpad.net/launchpad/+bug/911853

For more details, see:
https://code.launchpad.net/~rharding/launchpad/resizing_fix_911853/+merge/89036

= Summary =
When adding a bug from Firefox the first time you typed into the box it jumped in size.

== Proposed Fix ==
Firefox wasn't keeping the min height setting because it was hidden beneath a div set to display: none.

== Implementation Details ==
When we run resize, make sure that the min height is always set by default for multi line inputs.

== Tests ==
google-chrome lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html

== Demo and Q/A ==
Add a new bug in Firefox. When the textarea becomes visible, make sure it's set to the default height of 300px. Typing into the box should not move/adjust until it's hit the limits of the initial height.

== Lint ==
Linting changed files:
  lib/lp/app/javascript/formwidgets/resizing_textarea.js
  lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html
  lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js
-- 
https://code.launchpad.net/~rharding/launchpad/resizing_fix_911853/+merge/89036
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/resizing_fix_911853 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/formwidgets/resizing_textarea.js'
--- lib/lp/app/javascript/formwidgets/resizing_textarea.js	2011-12-21 07:54:40 +0000
+++ lib/lp/app/javascript/formwidgets/resizing_textarea.js	2012-01-18 13:53:28 +0000
@@ -276,15 +276,17 @@
             new_height = Math.max(
                 this.get('min_height'),
                 Math.min(scroll_height, this.get('max_height')));
-
             this.t_area.setStyle('height', new_height);
-
-            // Check if the changes above require us to change our overflow
-            // setting to allow for a scrollbar now that our max size has been
-            // reached.
             this.set_overflow();
 
             this._prev_scroll_height = scroll_height;
+        } else if (this._prev_scroll_height !== this.get('min_height')) {
+            // this can occur when we go from hidden to not hidden via a
+            // parent display setting. The initial height needs to get hit
+            // again manually on display.
+            this.t_area.setStyle('height', this.get('min_height'));
+            this.set_overflow();
+            this._prev_scroll_height = this.get('min_height');
         }
     },
 

=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html'
--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html	2011-12-05 18:51:48 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html	2012-01-18 13:53:28 +0000
@@ -27,6 +27,11 @@
     <textarea id="multiple1" class="test_multiple first" style="width: auto;"></textarea>
     <textarea id="multiple2" class="test_multiple second" style="width: auto;"></textarea>
     <textarea id="css_height" style="height: 120px; width: auto;"></textarea>
+    <div style="display: none;">
+        <textarea id="config_height" rows="15" name="field.comment"
+            style="max-width: 60em; width: 90%; display: block;" cols="44">
+        </textarea>
+    </div>
     <textarea id="one_line_sample" style="height: 1em; overflow: hidden; resize: none; width: auto;"></textarea>
     <textarea id="one_line" style="height: 1em; width: auto;"></textarea>
 </body>

=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js'
--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js	2011-12-07 19:37:13 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js	2012-01-18 13:53:28 +0000
@@ -153,6 +153,21 @@
             "The height should match the css property at 120px");
     },
 
+    test_initial_min_height_after_hidden: function() {
+        // if we pass in a min height, the text area should resize on init to
+        // that min height
+        var target = Y.one('#config_height');
+        target.plug(Y.lp.app.formwidgets.ResizingTextarea, {
+            skip_animations: true,
+            min_height: 300
+        });
+
+        target.show();
+        var current_height = get_height(target);
+        Y.Assert.areSame(300, current_height,
+            "The height should start out at 300px per the min_height config");
+    },
+
     test_oneline_should_size_to_single_em: function() {
         // Passing a one line in the cfg should limit the height to 1em even
         // though a normal textarea would be two lines tall.