fix(input): [input] fixed the error when using v-if to switch back and forth after configuring the displayOnly attribute #2519
Conversation
…d forth after configuring the displayOnly attribute
WalkthroughThe pull request introduces modifications to two main files: Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
commitlint.config.js (1)
3-5: Consider the implications of increasing header length limitThe header length limit of 150 characters is significantly higher than the conventional 100-character limit. While this provides more flexibility, it might lead to less concise and harder-to-read commit messages. Consider:
- Using a stricter limit (100-120 characters)
- Setting severity to 2 (error) instead of 1 (warning) to enforce the limit
rules: { - 'header-max-length': [1, 'always', 150] + 'header-max-length': [2, 'always', 100] }packages/renderless/src/input/index.ts (1)
Line range hint
273-297: Consider additional defensive programming improvementsWhile the null check for
vm.$refs.textareais good, consider these additional improvements for better type safety and error handling:
- Add type checking for the
autosizeproperty- Add boundary checks for minRows and maxRows
Here's a suggested improvement:
if (type !== 'textarea' || !vm.$refs.textarea) { return } if (props.hoverExpand && !state.enteredTextarea) { state.textareaCalcStyle = { minHeight: state.textareaHeight, height: state.textareaHeight } return } - if (!autosize) { + if (!autosize || typeof autosize !== 'object') { state.textareaCalcStyle = { minHeight: api.calcTextareaHeight(vm.$refs.textarea).minHeight } return } - const minRows = autosize.minRows - const maxRows = autosize.maxRows + const minRows = Math.max(1, Number(autosize.minRows) || 1) + const maxRows = Math.max(minRows, Number(autosize.maxRows) || Infinity)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
commitlint.config.js(1 hunks)packages/renderless/src/input/index.ts(1 hunks)
🔇 Additional comments (2)
commitlint.config.js (1)
1-6: Verify the relationship between this change and the PR objective
This configuration change appears unrelated to the PR's stated objective of fixing input component issues. Consider submitting this change as a separate PR to maintain clear change boundaries and facilitate easier review and tracking.
Let's verify if there are any recent commits that would justify this header length increase:
packages/renderless/src/input/index.ts (1)
273-273: Good defensive programming!
The additional check || !vm.$refs.textarea is a crucial fix that prevents potential null reference errors when the textarea element is unmounted by v-if directives. This addresses the core issue mentioned in the PR objectives where switching back and forth with v-if was causing errors.
Let's verify the component's usage of v-if with textarea:
✅ Verification successful
The search results confirm that textarea elements are conditionally rendered using v-if directives across multiple Vue components, validating the need for the defensive null check. Let me verify one more aspect to ensure there aren't other similar ref checks that might need the same treatment.
Based on the search results, I can now provide a definitive response about the textarea ref check:
The null check for vm.$refs.textarea is correctly implemented and verified!
The search results confirm that:
- The textarea element is conditionally rendered with v-if across multiple Vue components
- All other usages of
vm.$refs.textareain the codebase are already protected by either:- Being inside the same conditional block that checks for its existence
- Being in methods that are only called when the textarea is guaranteed to exist
The defensive null check is a crucial fix that prevents null reference errors when the textarea is unmounted, particularly during v-if directive transitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for v-if usage with textarea in component templates
rg -g '*.vue' 'v-if.*?textarea|textarea.*?v-if'
# Search for potential similar refs that might need the same defensive check
ast-grep --pattern 'if ($_ !== "textarea") {
$$$
}'
Length of output: 1080
Script:
#!/bin/bash
# Search for refs usage with textarea
rg -A 2 '\$refs\.textarea'
# Search for other potential ref usages that might need null checks
ast-grep --pattern '$_.$refs.$_'
Length of output: 60437
…d forth after configuring the displayOnly attribute
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit