Implement the log retention setting #7951
Conversation
CoderDake
force-pushed
the
logretention-model2
branch
from
e1b3641
to
0ae68c0
Compare
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_model.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_model.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_model.dart
Show resolved
Hide resolved
| @@ -49,9 +51,10 @@ class LoggingTableRow extends StatefulWidget { | |||
| // TODO(danchevalier): Improve row2 height by manually flowing metadas into another row | |||
| // if theyoverflow. | |||
| final row2Height = calculateTextSpanHeight( | |||
| TextSpan(text: text, style: metadataStyle), | |||
| TextSpan(text: 'short text for now', style: metadataStyle), | |||
There was a problem hiding this comment.
remove this hard coded string
There was a problem hiding this comment.
Metadata will be handled in a future PR. I'll just leave this for now.
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_table_v2.dart
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_table_v2.dart
Outdated
Show resolved
Hide resolved
| controller: TextEditingController( | ||
| text: newRetentionLimit.toString(), | ||
| ), |
There was a problem hiding this comment.
this will get created on each build and never disposed, causing memory leaks. Create the text editing controller in initState and use it here. Also dispose it in dispose.
By doing this, I think we can also drop the newRetentionLimit variable, since you can just lookup the text from the controller textEditingController.text
There was a problem hiding this comment.
updated, but it doesn't look like it has a dispose function.
There was a problem hiding this comment.
It should. Here is an example of another TextEditingController getting disposed: https://github.com/flutter/devtools/blob/master/packages/devtools_app/lib/src/framework/home_screen.dart/#L191
packages/devtools_app/lib/src/shared/preferences/_logging_preferences.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/test/logging/logging_screen_v2/logging_model_test.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_table_v2.dart
Outdated
Show resolved
Hide resolved
| SizedBox( | ||
| height: defaultTextFieldHeight, | ||
| width: defaultTextFieldNumberWidth, | ||
| child: TextField( |
There was a problem hiding this comment.
looking at the screenshot, the text is not vertically centered in the text box. Can we fix that as part of this change?
There was a problem hiding this comment.
To get it closer to vertically centered, I've set it to TextAlignVertical.top
a
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_model.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_table_v2.dart
Outdated
Show resolved
Hide resolved
| _textEditingController = TextEditingController() | ||
| ..text = widget.notifier.value.toString(); |
There was a problem hiding this comment.
does this text controller need to be updated when the value of widget.notifier changes? for example if widget.notifier changed outside the context of this widget?
CoderDake
added
the
autosubmit
Implement the log retention setting.
