Merge lp:~rharding/launchpad/resizing_textarea_919299 into lp:launchpad

Proposed by Richard Harding on 2012-01-21
Status: Merged
Approved by: Curtis Hovey on 2012-01-21
Approved revision: no longer in the source branch.
Merged at revision: 14713
Proposed branch: lp:~rharding/launchpad/resizing_textarea_919299
Merge into: lp:launchpad
Diff against target: 61 lines (+28/-1)
3 files modified
lib/lp/app/javascript/formwidgets/resizing_textarea.js (+1/-1)
lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html (+1/-0)
lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js (+26/-0)
To merge this branch: bzr merge lp:~rharding/launchpad/resizing_textarea_919299
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code 2012-01-21 Approve on 2012-01-21
Review via email: mp+89515@code.launchpad.net

Commit Message

[r=sinzui][bug=919299] Correct condition for setting resizing textarea to min height configured.

Description of the Change

= Summary =
There was an issue with the last bug fix that caused the 3rd change to a resizing textarea to resize to the min height for the widget. This then was reverted on the next resize event, and repeated over and over which has a lovely bouncing textarea effect.

== Proposed Fix ==
Correct the condition when we should make sure the textarea changes itself to the min height configured.

== Implementation Details ==
Basically the check should have been < and not !=

== Tests ==
Added a new test to verify the condition

google-chrome lib/lp/app/javascript/tests/test_beta_notification.html

== Demo and Q/A ==
Enter enough text into an inline edit widget that it hits the max size. Then make sure that it sticks there vs reverting between the max and min heights configured.

== 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

To post a comment you must log in.
Curtis Hovey (sinzui) wrote :

Thank you for this fix.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/javascript/formwidgets/resizing_textarea.js'
2--- lib/lp/app/javascript/formwidgets/resizing_textarea.js 2012-01-18 13:54:33 +0000
3+++ lib/lp/app/javascript/formwidgets/resizing_textarea.js 2012-01-21 00:46:25 +0000
4@@ -280,7 +280,7 @@
5 this.set_overflow();
6
7 this._prev_scroll_height = scroll_height;
8- } else if (this._prev_scroll_height !== this.get('min_height')) {
9+ } else if (this._prev_scroll_height < this.get('min_height')) {
10 // This can occur when we go from hidden to not hidden via a
11 // parent display setting. The initial height needs to get hit
12 // again manually on display.
13
14=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html'
15--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html 2012-01-18 13:46:13 +0000
16+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.html 2012-01-21 00:46:25 +0000
17@@ -32,6 +32,7 @@
18 style="max-width: 60em; width: 90%; display: block;" cols="44">
19 </textarea>
20 </div>
21+ <textarea id="no_change" style="width: auto;"></textarea>
22 <textarea id="one_line_sample" style="height: 1em; overflow: hidden; resize: none; width: auto;"></textarea>
23 <textarea id="one_line" style="height: 1em; width: auto;"></textarea>
24 </body>
25
26=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js'
27--- lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js 2012-01-18 13:54:33 +0000
28+++ lib/lp/app/javascript/formwidgets/tests/test_resizing_textarea.js 2012-01-21 00:46:25 +0000
29@@ -168,6 +168,32 @@
30 "The height should start out at 300px per the min_height config");
31 },
32
33+ test_height_stays_consistant: function () {
34+ // Once we adjust the height, another keystroke shouldn't move the
35+ // height on us again, see bug #919299.
36+ var target = Y.one('#no_change');
37+ target.plug(Y.lp.app.formwidgets.ResizingTextarea, {
38+ skip_animations: true,
39+ max_height: 200,
40+ min_height: 100
41+ });
42+
43+ update_content(target, test_text);
44+ var new_height = get_height(target);
45+ Y.Assert.areSame(200, new_height,
46+ "The height should hit max at 200px");
47+
48+ update_content(target, test_text + "3");
49+ var adjusted_height = get_height(target);
50+ Y.Assert.areSame(200, adjusted_height,
51+ "The height should still be at 200px");
52+
53+ update_content(target, test_text + "34");
54+ var adjusted_height2 = get_height(target);
55+ Y.Assert.areSame(200, adjusted_height2,
56+ "The height should still be at 200px");
57+ },
58+
59 test_oneline_should_size_to_single_em: function() {
60 // Passing a one line in the cfg should limit the height to 1em even
61 // though a normal textarea would be two lines tall.