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

feat(TextArea): add minHeight property, docs example #1679

Merged
merged 5 commits into from
Jun 7, 2017

Conversation

josie11
Copy link
Contributor

@josie11 josie11 commented May 14, 2017

Adds minHeight property to TextArea that can be used with autoHeight property to specify a minimum number of rows a TextArea should have. Referenced here: #1647.

I've implemented this by setting this.ref.rows = minHeight, but I'm not sure if this is the ideal way. I'd have to adjust assertHeight in the tests in order to add a test for it.

@codecov-io
Copy link

codecov-io commented May 14, 2017

Codecov Report

Merging #1679 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1679      +/-   ##
==========================================
- Coverage   99.75%   99.75%   -0.01%     
==========================================
  Files         142      142              
  Lines        2468     2466       -2     
==========================================
- Hits         2462     2460       -2     
  Misses          6        6
Impacted Files Coverage Δ
src/addons/TextArea/TextArea.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d41235c...a26a740. Read the comment docs.

@layershifter layershifter changed the title Feat(TextArea) add minHeight property, docs example feat(TextArea): add minHeight property, docs example May 15, 2017
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Thanks for PR. I think that rows should be also customizable:

static defaultProps = {
   as: 'textarea',
+  minHeight: 0,
+  rows: 5,
}
-this.ref.rows = minHeight
-this.ref.style.minHeight = '0'
+this.ref.style.minHeight = minHeight + 'px'
render() {
-  const { value } = this.props
+  const { rows, value } = this.props
// ...
- return <ElementType{...rest} onChange={this.handleChange} ref={this.handleRef} value={value} />
+  return (
+    <ElementType
+      {...rest} 
+      onChange={this.handleChange}
+      ref={this.handleRef}
+      rows={rows}
+      value={value}
+    />
+  )
}

@josie11
Copy link
Contributor Author

josie11 commented May 15, 2017

I added the rows prop but I left it in updateHeight & usable with autoHeight for now - otherwise it will break tests. Also, when using autoHeight it seems the intended default rows should be 1, so there is a conflict there.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

I've pushed some changes there, LGTM.

However, there is one breaking change with autoHeight. Before these changes rows prop had 1 value, now it's has 3 by default.

@levithomason
Copy link
Member

Reviewing the code changes, it strikes me odd that we're pulling autoHeight up to a prop. Perhaps it makes more sense to leave this in the style prop:

<TextArea style={{ minHeight: 100 }} />

@layershifter
Copy link
Member

@levithomason Thanks for the feedback, I'll make these changes.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

I've performed there some changes. minHeight was back to style 👍

Copy link
Member

@levithomason levithomason left a comment

Choose a reason for hiding this comment

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

Thanks much @josie11 and @layershifter!

@levithomason levithomason merged commit 01bdd6f into Semantic-Org:master Jun 7, 2017
aabustamante added a commit to aabustamante/Semantic-UI-React that referenced this pull request Jun 15, 2017
* docs(Button): remove redundant prop in Vertical Group example (Semantic-Org#1699)

* feat(typescript): Export generics types (Semantic-Org#1698)

* docs(Dropdown): fix world icon in search example (Semantic-Org#1695)

* Fix MenuExampleText

* add world icon to Dropdown example

* rename 'languageOptions' to 'languages'

* rename  languages   to languageOptions and separate line into two

* separate line of code in two

* separate props in different lines

* remove trailing spaces

* docs(Introduction): fix declarative example (Semantic-Org#1704)

* feat(Item): add unstackable prop to ItemGroup (Semantic-Org#1706)

* feat(Item): add unstackable prop to ItemGroup

* docs(Item): add example for unstackable

* chore(package): commit package-lock.json

* 0.68.4

* docs(changelog): update changelog [ci skip]

* docs(Icon): fix selector for input (Semantic-Org#1714)

* chore(package): update require-dir to version 0.3.2 (Semantic-Org#1721)

https://greenkeeper.io/

* docs(ItemExampleFloated): your description (Semantic-Org#1719)

Fixes error in docs. "AS" is not being recognized.

* chore(package): update react-ace to version 5.0.1 (Semantic-Org#1712)

https://greenkeeper.io/

* fix(Dropdown): add addition item key (Semantic-Org#1727)

* fix(factories): handle falsy `key` values (Semantic-Org#1729)

* fix(Dropdown): fix key handling

* fix(Dropdown): fix key handling

* fix(factories): handle falsy keys

* chore(package): update package-lock.json

* chore(package): update chai-enzyme to 0.7.1 (Semantic-Org#1731)

* feat(typings): expose FormComponent in typings (Semantic-Org#1680)

* Exposed form component in typings to support custom controls in packages.

* Update index.d.ts

* Update index.d.ts

* chore(package): update package-lock.json

* 0.68.5

* docs(changelog): update changelog [ci skip]

* fix(Input): add missing minLength prop (Semantic-Org#1734)

* docs(TableExampleSortable): pass in null when that column shouldn't be sorted (Semantic-Org#1737)

* feat(TextArea): add minHeight property, docs example (Semantic-Org#1679)

* add minHeight property, example to docs

* add rows prop/adjust minHeight prop usage

* mixed(TextArea): update docs, tests, props and typings

* fix(Textarea): move back minHeight prop to style

* fix(htmlInputProps): fix handle on falsy values (Semantic-Org#1746)

* fix(Search): Allow default action if there is no selected result (Semantic-Org#1742)

* docs(images): add missing images, update urls (Semantic-Org#1763)

* fix(Accordion): typings inverted to boolean (Semantic-Org#1758)

Tiny fix for typings, inverted type incorrectly set to string.

* feat(Icon): add ability use the loading prop without an icon (Semantic-Org#1768)

* feat(Button): add focus method (Semantic-Org#1764)

* fix(Dropdown): change active item on keyboard up/down (Semantic-Org#1735)

* fix(Dropdown): change active item on keyboard up/down

* fix(Dropdown): change active item on keyboard up/down

* refactor(Dropdown): simplify move constant

* refactor(Dropdown): remove hidden select (Semantic-Org#1730)

breaking(Dropdown): remove hidden select

* fix(Checkbox|Input): fix handling of aria-attributes (Semantic-Org#1752)

* fix(Checkbox|Input): fix handling of aria-attributes

* feat(htmlInputProps): update handling of aria
@levithomason
Copy link
Member

Released in semantic-ui-react@0.69.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants