feat: add JSON format button in apollo-portal #5287
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe pull request introduces several enhancements and fixes for Apollo version 2.4.0, including new features for server configuration, REST template client connection pooling, and global search capabilities. It also implements limits on namespace counts and adds JSON formatting support in the Apollo portal. The changes include updates to internationalization files for English and Chinese, new utility functions for formatting JSON content in directives, and a new button for formatting within the namespace panel. Additionally, the update addresses multiple fixes and refactoring efforts, including security improvements. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 2
🧹 Outside diff range and nitpick comments (4)
CHANGES.md (1)
25-25: Consider enhancing the changelog entry description.While the entry follows the correct format, consider making it more descriptive to better reflect the functionality and user benefit:
-* [Feature: add JSON format in apollo-portal](https://github.com/apolloconfig/apollo/pull/5287) +* [Feature: add JSON formatting capability to improve configuration readability in apollo-portal](https://github.com/apolloconfig/apollo/pull/5287)apollo-portal/src/main/resources/static/views/component/item-modal.html (1)
68-70: Enhance accessibility for the format link.The format link should include proper ARIA attributes for better accessibility.
Consider applying this improvement:
-<a ng-show="item.type == '3'" ng-click="formatContent()">{{'Component.ConfigItem.FormatItemValue' | translate }}</a> +<a ng-show="item.type == '3'" + ng-click="formatContent()" + role="button" + aria-label="{{'Component.ConfigItem.FormatItemValue' | translate }}" + title="{{'Component.ConfigItem.FormatItemValue' | translate }}">{{'Component.ConfigItem.FormatItemValue' | translate }}</a>apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js (1)
248-257: Consider architectural improvements and add unit tests.
- The JSON formatting logic could be moved to a shared service to promote reuse across different components.
- As mentioned in the PR objectives, unit tests are missing. Consider adding tests for:
- Formatting valid JSON
- Handling invalid JSON
- Handling empty/null values
- Preserving original value on error
Would you like me to help with:
- Creating a shared service for JSON formatting?
- Generating unit test cases?
apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html (1)
193-199: LGTM! The format button implementation is well integrated.The implementation follows the existing patterns for button styling, placement, and behavior. Good use of:
- Conditional rendering with appropriate checks
- Internationalization support
- Consistent styling with other buttons
- Tooltip for better UX
Consider adding keyboard shortcut support for the format action to improve accessibility. For example:
<img src="img/brush.png" class="ns_btn cursor-pointer" data-tooltip="tooltip" data-placement="bottom" title="{{'Component.ConfigItem.FormatItemValue' | translate }}" ng-show="namespace.isTextEditing && namespace.viewType == 'text' && namespace.format == 'json'" - ng-click="formatContent(namespace)"> + ng-click="formatContent(namespace)" + role="button" + tabindex="0" + ng-keydown="$event.keyCode === 13 && formatContent(namespace)">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
apollo-portal/src/main/resources/static/img/brush.pngis excluded by!**/*.png
📒 Files selected for processing (7)
CHANGES.md(1 hunks)apollo-portal/src/main/resources/static/i18n/en.json(1 hunks)apollo-portal/src/main/resources/static/i18n/zh-CN.json(1 hunks)apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js(2 hunks)apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js(2 hunks)apollo-portal/src/main/resources/static/views/component/item-modal.html(1 hunks)apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apollo-portal/src/main/resources/static/i18n/en.json
- apollo-portal/src/main/resources/static/i18n/zh-CN.json
🔇 Additional comments (5)
apollo-portal/src/main/resources/static/views/component/item-modal.html (1)
68-70: LGTM! Well-placed formatting option for JSON content.
The formatting link is logically placed and correctly shown only for JSON type items (type='3'). The internationalization support ensures proper localization of the feature.
apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js (1)
44-44: LGTM!
The formatContent function is correctly exposed to the directive's scope, following AngularJS best practices.
apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js (2)
89-89: LGTM! Appropriate scope function exposure
The function is correctly exposed to the directive's scope, maintaining consistency with other function declarations in the file.
738-747: Verify the template integration of formatContent
Let's ensure the function is properly integrated with the UI components.
✅ Verification successful
formatContent function is properly integrated with the UI
The function is correctly integrated in the template with a dedicated format button that appears when editing text in JSON format. The button is properly bound to the formatContent function through ng-click directive and has appropriate visibility conditions:
- Button appears only when:
- Text editing is active (
namespace.isTextEditing) - View type is text (
namespace.viewType == 'text') - Format is JSON (
namespace.format == 'json')
- Text editing is active (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find the template file and verify the format button integration
# Search for the template file
echo "Searching for namespace panel template..."
fd "namespace-panel.html" --type f
# Search for formatContent usage in HTML files
echo "Checking formatContent usage in templates..."
rg -t html "formatContent"
Length of output: 693
Script:
#!/bin/bash
# Let's check the content of these files to understand the integration better
echo "Checking namespace-panel.html content..."
cat "apollo-portal/src/main/resources/static/views/component/namespace-panel.html"
echo -e "\nChecking namespace-panel-master-tab.html content..."
cat "apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html"
# Let's also check the directive file to see the complete context
echo -e "\nChecking namespace-panel-directive.js content..."
cat "apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js"
Length of output: 130961
apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html (1)
193-199: Verify required resources and dependencies.
Let's verify the existence of required resources:
- Translation key:
Component.ConfigItem.FormatItemValue - Image asset:
img/brush.png - Function:
formatContent(namespace)
✅ Verification successful
All required resources and dependencies are present
All necessary components have been verified:
- Translation key
Component.ConfigItem.FormatItemValueexists in both en.json and zh-CN.json - Image asset
brush.pngis present at the correct location formatContent(namespace)function is properly implemented innamespace-panel-directive.jswith JSON formatting functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for translation key
echo "Checking translation files..."
fd -e json . | xargs rg "Component\.ConfigItem\.FormatItemValue"
# Check for brush icon
echo "Checking for brush icon..."
fd "brush.png" .
# Check for formatContent function
echo "Checking for formatContent implementation..."
rg "formatContent.*function" -A 5
Length of output: 624
Script:
#!/bin/bash
# Let's try a different approach to find formatContent function
# Search for formatContent in JavaScript files
echo "Searching for formatContent in JS files..."
rg "formatContent" -t js -B 2 -A 5
# Also search in controller files specifically
echo "Searching in controller files..."
fd "controller.js" | xargs rg "formatContent" -B 2 -A 5
# Search in namespace related files
echo "Searching in namespace related files..."
rg "formatContent" --type-add 'web:*.{js,html}' -t web -g '*namespace*' -B 2 -A 5
Length of output: 7697
apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js
Outdated
Show resolved
Hide resolved
WTIFS
changed the title
There was a problem hiding this comment.
Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 suggestion.
Files not reviewed (4)
- apollo-portal/src/main/resources/static/i18n/en.json: Language not supported
- apollo-portal/src/main/resources/static/i18n/zh-CN.json: Language not supported
- apollo-portal/src/main/resources/static/views/component/item-modal.html: Language not supported
- apollo-portal/src/main/resources/static/views/component/namespace-panel-master-tab.html: Language not supported
apollo-portal/src/main/resources/static/scripts/directive/item-modal-directive.js
Show resolved
Hide resolved
| // 格式化 | ||
| function formatContent() { | ||
| if (scope.showJsonError) { | ||
| return |
apollo-portal/src/main/resources/static/scripts/directive/namespace-panel-directive.js
Outdated
Show resolved
Hide resolved
What's the purpose of this PR
Add a JSON format button under the configuration value textarea.
The reason is when user uses compact JSON format, all the content is inline and it's hard to find the differences when publishing. By formatting the content into separated lines, it's easier to read for human.
Which issue(s) this PR fixes:
Fixes #5285
Brief changelog
format linkinitem-modal.htmlandformatContentfunction initem-modal-directive.jsformat buttoninnamespace-panel-master-tab.htmlandformatContentfunction innamespace-panel-directive.jsFollow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
Release Notes for Apollo Version 2.4.0
New Features
Bug Fixes
Documentation