Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds automatic textarea resizing functionality to improve the user experience when entering content in project-related forms. The changes introduce a new Stimulus controller that dynamically adjusts textarea height based on content while respecting minimum and maximum row constraints.
Changes:
- Created a new
auto_resize_textarea_controller.jsStimulus controller to handle automatic textarea resizing - Updated three form templates (thoughts, resources, and journal entries) to use the new auto-resize functionality
- Changed CSS from
resize-nonetoresize-yto allow both automatic and manual resizing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/javascript/controllers/auto_resize_textarea_controller.js | New Stimulus controller implementing auto-resize logic with configurable min/max rows |
| app/views/projects/thoughts/_form.html.erb | Added auto-resize controller integration with data attributes for min/max rows configuration |
| app/views/projects/resources/_form.html.erb | Added auto-resize controller integration with data attributes for min/max rows configuration |
| app/views/projects/journal_entries/_form.html.erb | Added auto-resize controller integration with data attributes for min/max rows configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export default class extends Controller { | ||
| static values = { | ||
| minRows: { type: Number, default: 2 }, | ||
| maxRows: { type: Number, default: 10 } |
There was a problem hiding this comment.
The default value for maxRows in the controller is set to 10, but all the view templates are explicitly passing 12. Consider updating the default to 12 in the controller to match the usage in the views, or document why the default differs from the common usage.
| maxRows: { type: Number, default: 10 } | |
| maxRows: { type: Number, default: 12 } |
| // Calculate new height based on content | ||
| const newHeight = Math.min(Math.max(textarea.scrollHeight, minHeight), maxHeight) | ||
| textarea.style.height = `${newHeight}px` | ||
|
|
||
| // Show scrollbar if content exceeds max height | ||
| textarea.style.overflowY = textarea.scrollHeight > maxHeight ? 'auto' : 'hidden' |
There was a problem hiding this comment.
The overflow check occurs after setting the textarea height to newHeight, which may cause scrollHeight to be inaccurate. Consider capturing the original scrollHeight before setting the new height, or check if the original scrollHeight exceeded maxHeight before applying the height constraint. For example, store 'const contentHeight = textarea.scrollHeight' before line 28, and then use 'contentHeight > maxHeight' in line 32.
| // Calculate new height based on content | |
| const newHeight = Math.min(Math.max(textarea.scrollHeight, minHeight), maxHeight) | |
| textarea.style.height = `${newHeight}px` | |
| // Show scrollbar if content exceeds max height | |
| textarea.style.overflowY = textarea.scrollHeight > maxHeight ? 'auto' : 'hidden' | |
| // Capture content height before applying max height constraint | |
| const contentHeight = textarea.scrollHeight | |
| // Calculate new height based on content | |
| const newHeight = Math.min(Math.max(contentHeight, minHeight), maxHeight) | |
| textarea.style.height = `${newHeight}px` | |
| // Show scrollbar if content exceeds max height | |
| textarea.style.overflowY = contentHeight > maxHeight ? 'auto' : 'hidden' |
| auto_resize_textarea_min_rows_value: 2, | ||
| auto_resize_textarea_max_rows_value: 12 | ||
| }, | ||
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> |
There was a problem hiding this comment.
The CSS class changed from 'resize-none' to 'resize-y', which allows manual resizing. However, the auto-resize controller also sets the height programmatically on input. This may cause confusing behavior where user-initiated manual resizing is overridden by automatic resizing. Consider whether 'resize-none' would be more appropriate to prevent conflict between manual and automatic resizing, or document that manual resizing is intentionally supported alongside auto-resizing.
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> | |
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-none" %> |
| auto_resize_textarea_min_rows_value: 2, | ||
| auto_resize_textarea_max_rows_value: 12 | ||
| }, | ||
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> |
There was a problem hiding this comment.
The CSS class changed from 'resize-none' to 'resize-y', which allows manual resizing. However, the auto-resize controller also sets the height programmatically on input. This may cause confusing behavior where user-initiated manual resizing is overridden by automatic resizing. Consider whether 'resize-none' would be more appropriate to prevent conflict between manual and automatic resizing, or document that manual resizing is intentionally supported alongside auto-resizing.
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> | |
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-none" %> |
| auto_resize_textarea_min_rows_value: 2, | ||
| auto_resize_textarea_max_rows_value: 12 | ||
| }, | ||
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> |
There was a problem hiding this comment.
The CSS class changed from 'resize-none' to 'resize-y', which allows manual resizing. However, the auto-resize controller also sets the height programmatically on input. This may cause confusing behavior where user-initiated manual resizing is overridden by automatic resizing. Consider whether 'resize-none' would be more appropriate to prevent conflict between manual and automatic resizing, or document that manual resizing is intentionally supported alongside auto-resizing.
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-y" %> | |
| class: "w-full rounded-2xl border border-slate-300/80 bg-white px-4 py-3 text-base text-slate-900 placeholder:text-slate-400 focus:border-blue-500 focus:outline-none focus:ring-4 focus:ring-blue-400/30 resize-none" %> |
1 participant
No description provided.