-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat(textarea): adjusting min-height of the textarea for ionic theme #30950
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
base: next
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Updates ion-textarea styling so the control’s minimum height better matches the configured rows (especially for rows="1"), with additional Ionic-theme-specific selector and spacing fixes.
Changes:
- Add a
data-attr-rowsattribute to the textarea inner wrapper to enable rows-based styling. - Update Ionic theme sizing/min-height behavior, plus tighten
auto-growselectors to excludeauto-grow="false". - Adjust Ionic theme textarea top margin behavior (and remove the old outline-only margin rule).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| core/src/components/textarea/textarea.tsx | Adds data-attr-rows on the wrapper for CSS-based rows sizing. |
| core/src/components/textarea/textarea.ionic.scss | Reworks Ionic theme min-height logic, auto-grow selectors, and label-placement spacing. |
| core/src/components/textarea/textarea.ionic.outline.scss | Removes outline-only textarea margin-top rule (spacing now handled elsewhere). |
| core/src/components/textarea/textarea.common.scss | Adds rows-related min-height override for certain host/class combinations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@rugoncalves https://github.com/ionic-team/ionic-framework/actions/runs/21918703311/job/63293046306?pr=30950 is failing in the PR checks |
brandyscarney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments requesting some test changes but I will defer to @thetaPC for the functionality since she has already left comments on that. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well spotted! That's indeed a bug!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove the shape, fill & label placement examples from this test. The only thing we really need to check for is things that will be affected by rows which is the default textareas, size and auto-grow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Accepting suggestion!
| <style> | ||
| .grid { | ||
| display: grid; | ||
| grid-template-columns: repeat(3, minmax(250px, 1fr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| grid-template-columns: repeat(3, minmax(250px, 1fr)); | |
| grid-template-columns: repeat(auto-fit, minmax(250px, 1fr)); |
| @media screen and (max-width: 800px) { | ||
| .grid { | ||
| grid-template-columns: 1fr; | ||
| padding: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @media screen and (max-width: 800px) { | |
| .grid { | |
| grid-template-columns: 1fr; | |
| padding: 0; | |
| } | |
| } |
See above change that accounts for this
| test('should respect rows attribute with fill outline and solid', async ({ page }) => { | ||
| await page.setContent( | ||
| ` | ||
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | ||
| <ion-textarea | ||
| rows="4" | ||
| fill="outline" | ||
| label="Outline" | ||
| label-placement="stacked" | ||
| helper-text="rows=4, fill=outline, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="4" | ||
| fill="solid" | ||
| label="Solid" | ||
| label-placement="stacked" | ||
| helper-text="rows=4, fill=solid, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| </div> | ||
| `, | ||
| config | ||
| ); | ||
|
|
||
| const container = page.locator('#container'); | ||
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-4-fill`)); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to check fill since it shouldn't modify anything related to the height.
Note: don't forget to remove screenshots
| test('should respect rows attribute with fill outline and solid', async ({ page }) => { | |
| await page.setContent( | |
| ` | |
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | |
| <ion-textarea | |
| rows="4" | |
| fill="outline" | |
| label="Outline" | |
| label-placement="stacked" | |
| helper-text="rows=4, fill=outline, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="4" | |
| fill="solid" | |
| label="Solid" | |
| label-placement="stacked" | |
| helper-text="rows=4, fill=solid, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| </div> | |
| `, | |
| config | |
| ); | |
| const container = page.locator('#container'); | |
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-4-fill`)); | |
| }); |
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-different-sizes`)); | ||
| }); | ||
|
|
||
| test('should respect rows attribute with value content', async ({ page }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this different from should respect rows attribute and set min-height?
| test('should respect rows attribute with different label placements', async ({ page }) => { | ||
| await page.setContent( | ||
| ` | ||
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | ||
| <ion-textarea | ||
| rows="3" | ||
| fill="outline" | ||
| label="Start" | ||
| label-placement="start" | ||
| helper-text="rows=3, fill=outline, label-placement=start" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| fill="outline" | ||
| label="End" | ||
| label-placement="end" | ||
| helper-text="rows=3, fill=outline, label-placement=end" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| fill="outline" | ||
| label="Floating" | ||
| label-placement="floating" | ||
| helper-text="rows=3, fill=outline, label-placement=floating" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| fill="outline" | ||
| label="Fixed" | ||
| label-placement="fixed" | ||
| helper-text="rows=3, fill=outline, label-placement=fixed" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| fill="outline" | ||
| label="Stacked" | ||
| label-placement="stacked" | ||
| helper-text="rows=3, fill=outline, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| </div> | ||
| `, | ||
| config | ||
| ); | ||
|
|
||
| const container = page.locator('#container'); | ||
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-label-placements`)); | ||
| }); | ||
|
|
||
| test('should respect rows attribute with different shapes', async ({ page }) => { | ||
| await page.setContent( | ||
| ` | ||
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | ||
| <ion-textarea | ||
| rows="3" | ||
| shape="soft" | ||
| fill="outline" | ||
| label="Soft" | ||
| label-placement="stacked" | ||
| helper-text="rows=3, shape=soft, fill=outline, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| shape="round" | ||
| fill="outline" | ||
| label="Round" | ||
| label-placement="stacked" | ||
| helper-text="rows=3, shape=round, fill=outline, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| <ion-textarea | ||
| rows="3" | ||
| shape="rectangular" | ||
| fill="outline" | ||
| label="Rectangular" | ||
| label-placement="stacked" | ||
| helper-text="rows=3, shape=rectangular, fill=outline, label-placement=stacked" | ||
| value="1 2 3 4 5 6 7 8 9 0" | ||
| ></ion-textarea> | ||
| </div> | ||
| `, | ||
| config | ||
| ); | ||
|
|
||
| const container = page.locator('#container'); | ||
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-shapes`)); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to check these since it shouldn't modify anything related to the height.
Note: don't forget to remove screenshots
| test('should respect rows attribute with different label placements', async ({ page }) => { | |
| await page.setContent( | |
| ` | |
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | |
| <ion-textarea | |
| rows="3" | |
| fill="outline" | |
| label="Start" | |
| label-placement="start" | |
| helper-text="rows=3, fill=outline, label-placement=start" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| fill="outline" | |
| label="End" | |
| label-placement="end" | |
| helper-text="rows=3, fill=outline, label-placement=end" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| fill="outline" | |
| label="Floating" | |
| label-placement="floating" | |
| helper-text="rows=3, fill=outline, label-placement=floating" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| fill="outline" | |
| label="Fixed" | |
| label-placement="fixed" | |
| helper-text="rows=3, fill=outline, label-placement=fixed" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| fill="outline" | |
| label="Stacked" | |
| label-placement="stacked" | |
| helper-text="rows=3, fill=outline, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| </div> | |
| `, | |
| config | |
| ); | |
| const container = page.locator('#container'); | |
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-label-placements`)); | |
| }); | |
| test('should respect rows attribute with different shapes', async ({ page }) => { | |
| await page.setContent( | |
| ` | |
| <div id="container" style="display: flex; flex-direction: column; gap: 20px;"> | |
| <ion-textarea | |
| rows="3" | |
| shape="soft" | |
| fill="outline" | |
| label="Soft" | |
| label-placement="stacked" | |
| helper-text="rows=3, shape=soft, fill=outline, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| shape="round" | |
| fill="outline" | |
| label="Round" | |
| label-placement="stacked" | |
| helper-text="rows=3, shape=round, fill=outline, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| <ion-textarea | |
| rows="3" | |
| shape="rectangular" | |
| fill="outline" | |
| label="Rectangular" | |
| label-placement="stacked" | |
| helper-text="rows=3, shape=rectangular, fill=outline, label-placement=stacked" | |
| value="1 2 3 4 5 6 7 8 9 0" | |
| ></ion-textarea> | |
| </div> | |
| `, | |
| config | |
| ); | |
| const container = page.locator('#container'); | |
| await expect(container).toHaveScreenshot(screenshot(`textarea-rows-shapes`)); | |
| }); |
| await expect(textarea).toHaveScreenshot(screenshot(`textarea-rows-3`)); | ||
| }); | ||
|
|
||
| test('should respect rows attribute with different values', async ({ page }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we combine this with should respect rows attribute and set min-height with the following:
rows=1
rows=3
rows=5 (or some other larger number)
and then just capture the same screenshots in different sizes? So there would be 3 tests:
// test 1
"small"
rows=1
rows=3
rows=5 (or some other larger number)
// test 2
"medium"
rows=1
rows=3
rows=5 (or some other larger number)
// test 3
"large"
rows=1
rows=3
rows=5 (or some other larger number)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented.



Issue number: resolves internal
What is the current behavior?
The

heightof the textarea would not correspond to the rows number, when it the row number was set to1:Ionic theme specifics:
:host([auto-grow]) .textarea-wrapper-innerThis selector was always being applied independently on the value of the attribute
auto-grow;What is the new behavior?
The

min-heightis now respecting the rows attribute:The ionic theme has the following changes:
.textarea-size-*classes stopped forcing themin-height;This will cause the last line to be partially visible;
:host([auto-grow]) .textarea-wrapper-inner, to be applied only whenauto-grow = false;--host-rowsin the host, so that the minimum height can be calculated in the elementdiv[.textarea-wrapper-inner];Does this introduce a breaking change?
Other information