Added new expansionDepth feature#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new expansionDepth parameter to the JsonInspector widget, allowing users to control how many levels of JSON hierarchy are expanded when initiallyExpanded is set to true. The feature addresses issue #1 and provides users with more granular control over the initial display state of complex JSON structures. Additionally, the PR includes several UI/UX enhancements such as copy buttons, tooltips, and an overflow menu with additional copy options.
Key changes:
- Added
expansionDepthparameter with default value of-1(expand all levels) - Updated example app with a settings menu to demonstrate depth control options
- Enhanced copy functionality with visual feedback and multiple copy options
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pubspec.yaml | Bumped version from 2.0.0 to 2.1.0 for the new feature release |
| lib/src/json_inspector_base.dart | Added expansionDepth parameter, depth-tracking logic in state initialization, and enhanced UI with copy buttons, tooltips, and overflow menu |
| example/lib/main.dart | Added expansion depth selector UI with popup menu and display of current expansion state |
| example/pubspec.lock | Updated transitive dependencies to newer versions |
| README.md | Documented the new expansionDepth parameter with usage examples |
| CHANGELOG.md | Added entry for version 2.1.0 with the new expansionDepth feature |
Comments suppressed due to low confidence (1)
example/lib/main.dart:135
- The interaction between the "Expand All/Collapse All" button and the expansion depth menu could be confusing. After selecting a specific depth (e.g., "2 levels"), clicking "Expand All" will expand to that depth rather than truly expanding all levels.
Consider one of these approaches:
- When clicking the "Expand All" button, reset
_expansionDepthto-1to ensure it truly expands all levels - Change the button text to dynamically reflect the current depth setting (e.g., "Expand 2 levels" instead of "Expand All" when a specific depth is selected)
- Add logic to differentiate between "expand all" and "expand to selected depth"
Current behavior: The "Expand All" button respects the previously selected depth, which may not match user expectations.
IconButton(
icon: Icon(_expandAll ? Icons.unfold_less : Icons.unfold_more),
onPressed: () {
setState(() {
_expandAll = !_expandAll;
});
},
tooltip: _expandAll ? 'Collapse All' : 'Expand All',
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// The depth level to expand when initiallyExpanded is true. | ||
| /// -1 means expand all levels, 1 means expand only first level, etc. | ||
| final int expansionDepth; |
There was a problem hiding this comment.
The expansionDepth parameter lacks validation for edge cases. Consider adding validation or documentation for:
expansionDepth = 0: Currently results in no expansion at all (even the root), which may be unintuitiveexpansionDepth < -1(e.g., -2, -3): Currently treated the same asexpansionDepth = 0(no expansion), which is inconsistent with the documented behavior where only-1means "expand all levels"
Consider either:
- Adding an assertion or validation to ensure
expansionDepth >= -1, or - Documenting these edge cases explicitly in the parameter documentation
| String _toJsonString(dynamic data) { | ||
| try { | ||
| return jsonEncode(data); | ||
| } catch (_) { | ||
| // Fallback for non-encodable objects | ||
| return data?.toString() ?? 'null'; | ||
| } | ||
| } | ||
|
|
||
| Future<void> _copyToClipboard(String text) async { | ||
| await Clipboard.setData(ClipboardData(text: text)); | ||
| } | ||
|
|
||
| void _showCopiedSnackBar(String message) { | ||
| final messenger = ScaffoldMessenger.maybeOf(context); | ||
| if (messenger == null) return; | ||
| messenger.hideCurrentSnackBar(); | ||
| messenger.showSnackBar( | ||
| SnackBar( | ||
| content: Text(message), | ||
| duration: const Duration(seconds: 1), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| Widget _buildOverflowMenu({ | ||
| required String path, | ||
| required String keyLabel, | ||
| required dynamic node, | ||
| }) { | ||
| return PopupMenuButton<String>( | ||
| tooltip: 'More actions', | ||
| itemBuilder: (context) => const [ | ||
| PopupMenuItem<String>( | ||
| value: 'copy_key', | ||
| child: Text('Copy key'), | ||
| ), | ||
| PopupMenuItem<String>( | ||
| value: 'copy_key_path', | ||
| child: Text('Copy key path'), | ||
| ), | ||
| PopupMenuItem<String>( | ||
| value: 'copy_json', | ||
| child: Text('Copy JSON'), | ||
| ), | ||
| ], | ||
| onSelected: (value) { | ||
| switch (value) { | ||
| case 'copy_key': | ||
| _copyToClipboard(keyLabel); | ||
| _showCopiedSnackBar('Copied key'); | ||
| break; | ||
| case 'copy_key_path': | ||
| final normalized = path.startsWith('/') ? path.substring(1) : path; | ||
| _copyToClipboard(normalized); | ||
| _showCopiedSnackBar('Copied key path'); | ||
| break; | ||
| case 'copy_json': | ||
| _copyToClipboard(_toJsonString(node)); | ||
| _showCopiedSnackBar('Copied JSON'); | ||
| break; | ||
| } | ||
| }, | ||
| child: const Padding( | ||
| padding: EdgeInsets.only(right: 4.0), | ||
| child: Icon(Icons.more_vert, size: 18), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
This PR introduces significant additional features beyond the expansionDepth parameter mentioned in the PR description. The changes include:
- Copy buttons with tooltips on all nodes
- An overflow menu with additional copy options (copy key, copy key path, copy JSON)
- A snackbar notification system for copy feedback
- JSON encoding improvements with the
_toJsonStringmethod
While these are valuable improvements, they should ideally be mentioned in the PR description and CHANGELOG for better tracking. Consider either:
- Updating the PR description and CHANGELOG to document all features added in this release, or
- Splitting these additional features into a separate PR for better version control and review
This is particularly important for version 2.1.0 as users reviewing the changelog will expect to see all new features documented.
|
@Protoss78 What do you think about the suggestion from copilot review? |
This PR would provide a new parameter that allows users to control the expansion depth, when initially expanded is set to true. Would address issue #1