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

add rows props to textarea #3

Closed
wants to merge 2 commits into from

Conversation

amazingandyyy
Copy link
Contributor

File changed:
index.js, Line 40

Reason:
make textarea look as a area

File changed:
   index.js, Line 40

Reason:
   make textarea look as a area
@dawsbot
Copy link
Owner

dawsbot commented May 11, 2017

  1. It looks on the same on my machine. Am I missing something?:

screen shot 2017-05-10 at 11 31 08 pm

  1. You don't need to verbally mention the line and number you changed, I can see that in the "Files changed" tab of the pull request. Save yourself the time and effort.

@dawsbot
Copy link
Owner

dawsbot commented May 11, 2017

I see it now. I don't think this is necessary. Convince me - do users often want multiple lines in their body sections?

@amazingandyyy
Copy link
Contributor Author

amazingandyyy commented May 11, 2017

I saw your last commit is to add the textarea which you may also agree a textarea is useful? The original idea is to fix the bugs(textarea height).

But after adding rows, I think it is necessary to be multiple lines.

--- my thinking is ---
If a user uses this helper tool, I assume he/she doesn't know how or are lazy to "encode" the content.
Making textarea as multiple lines can encourage the user to write a paragraph(with breaking lines) and when the user clicks Test Email, the result will surprise him.

This is a sweet moment and potentially makes user love this helper tool more.

Tell me how do you think. :)

@dawsbot
Copy link
Owner

dawsbot commented May 21, 2017

The textarea itself is adjustable and therefore does not seem to need a default row amount. If there is a default larger than 1, it pushes the "Use It" section off the top-fold of page content. If you disagree strongly @amazingandyyy, feel free to reopen this.

@dawsbot dawsbot closed this May 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants