-
Notifications
You must be signed in to change notification settings - Fork 571
Turning textfield multiline #828
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: main
Are you sure you want to change the base?
Conversation
…one as per instructions
@TeeWrath Hey, really appreciate your first contribution to API Dash — welcome aboard! .I noticed you added Just try typing a long text in the Google Search bar as @animator provides the example image — it expands while editing, then resets to its original size on focus loss. |
Thank you so much for such a warm welcome and your review @BalaSubramaniam12007 and @animator . I totally understand the importance of good UX and as per your suggestion, I have used the I have also set the To further smoothen the UX I have wrapped the below is a video demonstration of how it looks now: multiline.mp4Please let me know if any changes are required further. |
@TeeWrath yeah it looks good also when the send button is clicked then also it should go to normal state then it would be perfect |
@WannaCry016 @BalaSubramaniam12007 Should I move forward with a similar pattern for other TextFields too ? |
Test this feature in mobile. |
@animator @BalaSubramaniam12007 Hi, as per the directions given to me, I have implemented multi-line textfields for :
The fields revert back to 1 liners when not focused upon and are multi-line during editing, as per the desired UX. @animator on testing on mobile I found that this works on mobile too. I am attaching a screen recording here for the reference. multi-line.mobile.mp4Next I will implement similar multi-line functionality in the Request name text field in the collection pane. |
That’s great to hear! The output looks good to me as well. Just have a quick discussion with the mentors @animator sir @ashitaprasad mam and confirm the expected outcome and align on any final tweaks. |
With this commit, I have also turned the side pane search field to multi-line while following the pattern recommended. A preview : Current checklist:
I request @animator sir and @ashitaprasad mam to review this and give me further instructions if any for improvement. Thank you. |
In the demo video, for the URL it is fine, but the variable implementation is not useful .. what is the point of having it multiline if all the text content is not visible and you have to scroll vertically inside the text field. |
I was thinking the same, @TeeWrath . For your extra reference, please see how Postman handles this . |
@BalaSubramaniam12007 @animator Right sir, I will adjust this as per your suggestions. I will try to make it look similar to how it is on Postman. Have an exam tomorrow, will fix this after coming back. Thank you. |
…ne, need cursor and interaction yet
@animator hello sir, I have implemented an overlay kind of like it is in postman for the variable textfields. Would be great if you could give some reviews on it. Preview is below. var.mp4 |
@TeeWrath Test it on mobile and upload video. |
@animator I noticed a small glitch in the mobile version. Working on it. Currently I am occupied with some personal engagements, will promptly tweak this once I am done with it. Thanks for your patience. |
@TeeWrath Any updates? |
@animator sir, I'm traveling right now, have made a few changes and will push them as soon as I reach home. Most probably by the 19th if that is not a problem? Thank you sir. |
No problem @TeeWrath 👍 |
Nevermind @animator sir! I have pushed the changes. Below is the video. It works both on mobile and desktop. I request you to kindly review the changes and let me know if any changes are required further. Thank you mobfield.mp4 |
Hi @ashitaprasad mam and @animator sir, Just checking in on the status of this PR. It's been under review for three weeks. Any updates or feedback? Thanks! |
This PR is in the review queue. We are still on it. |
@animator @DenserMeerkat I have resolved the merge conflict. Kindly provide your valuable feedback. |
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
This PR adds multiline support to text fields throughout the API Dash app by converting StatelessWidget text field components to StatefulWidget. The implementation includes animated resizing when text fields gain focus and display multi-line content, along with overlay functionality for better multiline editing experience.
- Converts text field widgets from StatelessWidget to StatefulWidget to manage focus and multiline states
- Adds animated size transitions and overlay support for enhanced multiline editing
- Updates environment variable editing interface to support the new multiline behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
File | Description |
---|---|
packages/apidash_design_system/lib/widgets/textfield_raw.dart | Converts to StatefulWidget with focus tracking and animated multiline support |
packages/apidash_design_system/lib/widgets/textfield_outlined.dart | Major refactor to StatefulWidget with complex overlay system and multiline detection |
lib/widgets/field_cell.dart | Adds overlay callback support and wraps field in SizedBox |
lib/screens/envvar/editor_pane/variables_pane.dart | Implements overlay management system and updates data table configuration |
lib/screens/common_widgets/env_trigger_field.dart | Adds focus tracking and animated multiline behavior with null safety fixes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bool isFocused = false; | ||
@override | ||
void initState() { | ||
isFocused = false; |
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.
The isFocused field is already initialized to false at declaration (line 27), making this assignment in initState() redundant.
isFocused = false; |
Copilot uses AI. Check for mistakes.
hintStyle: hintTextStyle, | ||
contentPadding: kPv8, | ||
return AnimatedSize( | ||
duration: Duration(milliseconds: 200), |
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.
Use const constructor for Duration to improve performance: const Duration(milliseconds: 200)
.
duration: Duration(milliseconds: 200), | |
duration: const Duration(milliseconds: 200), |
Copilot uses AI. Check for mistakes.
TextStyle(), | ||
Theme.of(context).colorScheme, | ||
_focusNode, | ||
_controller, | ||
InputDecoration(), |
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.
Empty TextStyle() and InputDecoration() objects are passed to onOverlayToggle callback. Consider using meaningful values or make these parameters optional to avoid confusion.
Copilot uses AI. Check for mistakes.
final contentPadding = widget.contentPadding ?? const EdgeInsets.symmetric(horizontal: 8, vertical: 8); | ||
|
||
return RawKeyboardListener( | ||
focusNode: FocusNode(), |
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.
A new FocusNode is created on every build without proper disposal. This creates a resource leak. Consider reusing the existing _focusNode or create and manage the FocusNode properly.
Copilot uses AI. Check for mistakes.
}, | ||
onTap: () { | ||
if (!_isFocused) { | ||
FocusScope.of(context).requestFocus(FocusNode()); |
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.
Creating and requesting focus on an anonymous FocusNode before requesting focus on _focusNode is unnecessary and potentially problematic. Remove line 247 and only use _focusNode.requestFocus()
.
FocusScope.of(context).requestFocus(FocusNode()); |
Copilot uses AI. Check for mistakes.
), | ||
DataColumn2( | ||
label: Text("Variable value"), | ||
label: Text(" jars value"), |
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.
The label text ' jars value' appears to be a typo and should be 'Variable value' to match the original functionality.
label: Text(" jars value"), | |
label: Text("Variable value"), |
Copilot uses AI. Check for mistakes.
rows: dataRows, | ||
return GestureDetector( | ||
onTap: () { | ||
FocusScope.of(context).requestFocus(FocusNode()); |
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.
Creating a new FocusNode without disposal causes a resource leak. Use FocusScope.of(context).unfocus()
instead to clear focus.
FocusScope.of(context).requestFocus(FocusNode()); | |
FocusScope.of(context).unfocus(); |
Copilot uses AI. Check for mistakes.
onTapOutside: (event) { | ||
_focusNode.unfocus(); | ||
}, | ||
), |
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.
Missing readOnly and obscureText properties that were present in the original implementation. These properties should be passed to ExtendedTextField to maintain backward compatibility.
Copilot uses AI. Check for mistakes.
Hi @animator
PR Description
This is in reference to the issue #826 recently opened. I have started applying this the textfields present throughout the app. First I started with the text field in URL card. After my changes, it looks like this:
I will move forward with other textfield, while I would love to get reviews on this. I am contributing to API Dash for the first time, hence I'm planning to move ahead with reviews.
Next I will focus on:
EnvironmentTriggerField
which I already modified to multi-line for the url field.)Checklist
main
branch before making this PRflutter upgrade
and verify)flutter test
) and all tests are passingOS on which you have developed and tested the feature?