Fix to add missing URL encoding in drivers #19
Conversation
WalkthroughThe code refactors driver configuration in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
src/main/resources/META-INF/resources/frontend/src/fc-sharee-connector.js (1)
102-116: Great implementation that addresses the core URL encoding issue.This method effectively fixes the regression described in issue #18 by properly encoding URL parameters for the affected drivers. The use of
encodeURIComponent()is correct for all platforms.Minor formatting suggestion for the WhatsApp URL:
- return `https://wa.me?text=${encodeURIComponent(this.options?.shareText)} \n ${encodeURIComponent(this.options?.shareLink)}` + return `https://wa.me?text=${encodeURIComponent(this.options?.shareText)}\n${encodeURIComponent(this.options?.shareLink)}`The extra space before
\nand after\nmight not be necessary and could cause formatting issues in the WhatsApp message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/META-INF/resources/frontend/src/fc-sharee-connector.js(2 hunks)
🔇 Additional comments (3)
src/main/resources/META-INF/resources/frontend/src/fc-sharee-connector.js (3)
24-28: LGTM! Good refactoring to centralize driver configuration.The consolidation of driver configuration calls into
_configureDrivers()improves maintainability and follows the DRY principle.
30-37: LGTM! Consistent refactoring with thecreatemethod.The same centralization pattern is correctly applied here, maintaining consistency across both creation methods.
89-100: Excellent centralization with comprehensive documentation.The new
_configureDriversmethod effectively consolidates driver configuration logic. The JSDoc documentation clearly explains the purpose and temporary nature of these workarounds, which will help future maintainers.
|
Closing this in favor of #20. The new version of the base component includes the necessary enconding fixes. |
This PR brings back the helper method to update the missing encoding on some of the drivers links (see description in #18 for details).
It also includes a minor refactor to group configuration methods to avoid repetition.
Close #18
Summary by CodeRabbit
Bug Fixes
Refactor