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(form-textarea): add `noAutoShrink` prop (closes #2664) #2666

Merged
merged 17 commits into from Feb 23, 2019

Conversation

Projects
None yet
2 participants
@jackmu95
Copy link
Member

commented Feb 20, 2019

Description of Pull Request:

Currently the <b-form-textarea> resizes to min-height (or the content height, whichever is greater) even when it had a taller height before.
This PR fixes this misbehavior.

Add in a new prop that will allow users to disable the auto shrink to content height (i.e. making max height "sticky").

Closes #2664.

To Do:

  • docs update

PR checklist:

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Enhancement to an existing feature
  • ARIA accessibility
  • Documentation update
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact:

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (i.e.
    fixes #xxxx[,#xxxx], where "xxxx" is the issue number)
  • The PR should address only one issue or feature. If adding multiple features or fixing a bug
    and adding a new feature, break them into separate PRs if at all possible.
  • PR titles should following the
    Conventional Commits naming convention (i.e.
    "fix(alert): not alerting during SSR render", "docs(badge): Updated pill examples, fix typos",
    "chore: fix typo in docs", etc). This is very important, as the CHANGELOG is generated
    from these messages.

If new features/enhancement/fixes are added or changed:

  • Includes documentation updates (including updating the component's package.json for slot and
    event changes)
  • New/updated tests are included and passing (if required)
  • Existing test suites are passing
  • The changes have not impacted the functionality of other components or directives
  • ARIA Accessibility has been taken into consideration (does it affect screen reader users or
    keyboard only users? clickable items should be in the tab index, etc)

If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a
    suggestion issue first and wait for approval before working on it)
@codecov

This comment has been minimized.

Copy link

commented Feb 20, 2019

Codecov Report

Merging #2666 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             dev   #2666   +/-   ##
=====================================
  Coverage   70.9%   70.9%           
=====================================
  Files        173     173           
  Lines       3200    3200           
  Branches     938     938           
=====================================
  Hits        2269    2269           
  Misses       675     675           
  Partials     256     256
Impacted Files Coverage Δ
src/components/form-textarea/form-textarea.js 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 d2e1f8a...2a34147. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented Feb 20, 2019

Codecov Report

Merging #2666 into dev will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #2666      +/-   ##
=========================================
+ Coverage   71.9%   71.92%   +0.02%     
=========================================
  Files        173      173              
  Lines       3196     3199       +3     
  Branches     937      938       +1     
=========================================
+ Hits        2298     2301       +3     
  Misses       655      655              
  Partials     243      243
Impacted Files Coverage Δ
src/components/form-textarea/form-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 0edbc17...4012fed. Read the comment docs.

@jackmu95 jackmu95 requested a review from tmorehouse Feb 20, 2019

@jackmu95 jackmu95 added the Type: Bug label Feb 20, 2019

@tmorehouse

This comment was marked as resolved.

Copy link
Member

commented Feb 20, 2019

@jackmu95 Please note that original behaviour is the intended behaviour and is not a bug. The text area is supposed to grow and shrink to fit the content.. not get stuck to the maximum size/height encountered.

tmorehouse added some commits Feb 22, 2019

@jackmu95 jackmu95 changed the title fix(form-textarea): fix auto-height calculation (closes #2664) feat(form-textarea): add `noAutoShrink` prop (closes #2664) Feb 22, 2019

@jackmu95 jackmu95 requested a review from tmorehouse Feb 22, 2019

@tmorehouse

This comment was marked as resolved.

Copy link
Member

commented Feb 22, 2019

Adding in an extra check to handle the case when no max-rows is given and manual dragged resize is enabled.

tmorehouse added some commits Feb 22, 2019

@tmorehouse tmorehouse removed the Status: WIP label Feb 23, 2019

tmorehouse and others added some commits Feb 23, 2019

@jackmu95 jackmu95 requested a review from tmorehouse Feb 23, 2019

@jackmu95

This comment has been minimized.

Copy link
Member Author

commented Feb 23, 2019

@tmorehouse I've added some docs and for me the auto-height handling is completely broken. Can you confirm that in the deployment preview?

@tmorehouse

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

Ok will check here shortly.

jackmu95 and others added some commits Feb 23, 2019

@tmorehouse

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

@jackmu95 you were missing the max-rows props on the auto-height examples. I've added them in and the examples work now.

@jackmu95

This comment has been minimized.

Copy link
Member Author

commented Feb 23, 2019

@tmorehouse This removes the possibility to resize the textarea at all for me.

OSX 10.14.3
Chrome 72

@tmorehouse

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

@jackmu95 that is correct (as was/is documented). Using the auto-resize feature (which is triggered by setting max-rows), the resize handle is removed from the textarea via CSS.

Note that the resize handle of the textarea (if supported by the browser) will automatically be disabled in auto-height mode.

@jackmu95 jackmu95 merged commit a29c40c into bootstrap-vue:dev Feb 23, 2019

8 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 71.9%)
Details
codecov/project 71.92% (+0.02%) compared to 0edbc17
Details
deploy/netlify Deploy preview ready!
Details
security/snyk - package.json (pi0) No manifest changes detected

@jackmu95 jackmu95 deleted the jackmu95:fix-form-textarea-auto-height branch Feb 23, 2019

@tmorehouse tmorehouse referenced this pull request Feb 24, 2019

Merged

chore: release v2.0.0-rc.14 #2704

1 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.