Rename provider keys to avoid colission #15
Conversation
WalkthroughThe changes involve a comprehensive renaming of methods in the Changes
Poem
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: 0
Outside diff range and nitpick comments (1)
src/lib/index.js (1)
Line range hint
1-314: Overall LGTM! Consider updating documentation.The changes in this file successfully address the issue of provider key collisions by consistently prefixing all provided method names with 'tawk'. This modification aligns perfectly with the PR objective and maintains the existing functionality.
However, it's important to note that this change might affect existing codebases using this library. To ensure a smooth transition for current users:
- Consider updating the README or documentation to clearly communicate this change.
- You might want to add a migration guide for users updating from a previous version.
- It would be helpful to bump the major version number of the package, as this is a breaking change.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/lib/index.js (4 hunks)
Additional comments not posted (4)
src/lib/index.js (4)
73-111: LGTM! Consistent renaming of action methods.The changes in the
provideActionsmethod are consistent with the PR objective. All action methods have been correctly prefixed with 'tawk' to avoid provider key collisions. The underlying functionality remains unchanged, which is good.
118-155: LGTM! Consistent renaming of getter methods.The changes in the
provideGettersmethod are consistent with the PR objective. All getter methods have been correctly prefixed with 'tawk' to avoid provider key collisions. The underlying functionality remains unchanged, which is good.
Line range hint
164-282: LGTM! Consistent renaming of listener methods.The changes in the
provideListenersmethod are consistent with the PR objective. All listener methods have been correctly prefixed with 'tawk' to avoid provider key collisions. The underlying functionality remains unchanged, which is good. It's also correct that the event names inwindow.addEventListenercalls were not modified, as these are likely defined by the Tawk API.
289-311: LGTM! Consistent renaming of setter methods.The changes in the
provideSettersmethod are consistent with the PR objective. All setter methods have been correctly prefixed with 'tawk' to avoid provider key collisions. The underlying functionality remains unchanged, which is good.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (7)
docs/ssr-setup.md (3)
29-33: LGTM! Consider updating the surrounding documentation.The changes to rename the injected functions from 'onLoad' and 'toggle' to 'tawkOnLoad' and 'tawkToggle' are correctly implemented and align with the PR objective of adding the 'tawk' prefix to avoid naming collisions.
Consider updating the text above this code block to reflect the new function names. For example:
-## Instance property -To access the API functions, call the **$tawkMessenger** anywhere in your components. +## Injected functions +To access the API functions, inject the renamed functions ('tawkOnLoad', 'tawkToggle', etc.) in your components.
48-51: LGTM! Consider updating the surrounding documentation.The changes to rename the injected function from 'onLoad' to 'tawkOnLoad' are correctly implemented and align with the PR objective of adding the 'tawk' prefix to avoid naming collisions.
Consider updating the text above this code block to reflect the new function name. For example:
-## Event handling -You can listen on events emitted by the plugin, It must be exactly match the name used to listen -to that event. +## Event handling +You can listen to events emitted by the plugin using the renamed functions. The function names must exactly match the new names with the 'tawk' prefix.
Line range hint
1-58: Consider updating the entire document for consistency.While the code blocks have been correctly updated with the new function names, the rest of the document still refers to the old naming convention. This inconsistency could lead to confusion for users trying to implement the updated API.
Consider reviewing and updating the entire document to reflect the new naming convention. This may include:
- Updating the introduction to mention the 'tawk' prefix for all API functions.
- Revising any examples or explanations that refer to the old function names.
- Checking if the linked API reference document (api-reference.md) has also been updated with the new function names.
These changes will ensure that the documentation is fully consistent with the new API naming convention.
docs/api-reference.md (4)
Line range hint
649-680: LGTM: tawkVisitor section updated correctlyThe
tawkVisitorsection has been properly updated with the new method name. The code examples reflect this change correctly.Consider revising the following sentence for better clarity:
- Setting or changing the values after the widget script has been downloaded will not send the values to the dashboard. + Setting or changing the values after the widget script has been downloaded will not update the values on the dashboard.
Line range hint
871-906: LGTM: tawkGetWindowType section updated correctlyThe
tawkGetWindowTypesection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkGetWindowTypemethod.Consider revising the following sentence for better grammar:
- Returns the current widget type whether it's inline or embed. + Returns the current widget type, indicating whether it's inline or embed.Tools
LanguageTool
[uncategorized] ~872-~872: Possible missing comma found.
Context: ...etWindowType Returns the current widget type whether it’s inline or embed. `Composi...(AI_HYDRA_LEO_MISSING_COMMA)
Line range hint
1238-1261: LGTM: tawkOnBeforeLoaded section updated correctlyThe
tawkOnBeforeLoadedsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly.Consider revising the following sentence for better grammar:
- Returns a boolean value (true or undefined) indicating when plugin is initialize. + Returns a boolean value (true or undefined) indicating when the plugin is initialized.Tools
LanguageTool
[grammar] ~1239-~1239: Consider using either the past participle “initialized” or the present participle “initializing” here.
Context: ...or undefined) indicating when plugin is initialize.Composition API```html <script set...(BEEN_PART_AGREEMENT)
Line range hint
14-1690: Overall assessment: Comprehensive and consistent renamingThe changes in this file demonstrate a thorough and consistent implementation of the renaming strategy, adding the "tawk" prefix to all API methods. This change affects the entire API surface, including method names, code examples, and documentation. The modifications have been applied correctly across all sections of the document.
Key points:
- All API methods have been renamed with the "tawk" prefix.
- Code examples for both Composition API and Options API have been updated accordingly.
- The overall structure and functionality of the API remain unchanged.
- Minor grammatical improvements have been suggested in a few sections.
These changes will result in a breaking change for existing users of the API, as noted in the PR objectives. It's crucial to:
- Update the changelog to clearly communicate these breaking changes.
- Provide migration guidelines for users updating from the previous version.
- Update any other related documentation or code samples that may reference the old method names.
Consider creating a separate migration guide document that outlines the changes and provides step-by-step instructions for updating existing code to use the new API method names.
Tools
LanguageTool
[uncategorized] ~872-~872: Possible missing comma found.
Context: ...etWindowType Returns the current widget type whether it’s inline or embed. `Composi...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/api-reference.md (79 hunks)
- docs/spa-setup.md (1 hunks)
- docs/ssr-setup.md (2 hunks)
Additional context used
LanguageTool
docs/api-reference.md
[uncategorized] ~418-~418: Possible missing article found.
Context: ...geSystem Callback function invoked when message is sent by the system. The message is p...(AI_HYDRA_LEO_MISSING_A)
[style] ~534-~534: Since a “visitor” is neither male nor female, the better pronoun might be “their”.
Context: ...voked when the visitor manually changes his name. The visitorName is passed to the ...(SOMEBODY_NEUTRAL)
[uncategorized] ~872-~872: Possible missing comma found.
Context: ...etWindowType Returns the current widget type whether it’s inline or embed. `Composi...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~1239-~1239: Consider using either the past participle “initialized” or the present participle “initializing” here.
Context: ...or undefined) indicating when plugin is initialize.Composition API```html <script set...(BEEN_PART_AGREEMENT)
Additional comments not posted (18)
docs/spa-setup.md (1)
57-60: LGTM! Changes align with PR objectives.The modifications in the Options API example correctly implement the 'tawk' prefix for the event handling function, which is consistent with the PR objectives to avoid provider key collisions.
docs/api-reference.md (17)
14-33: LGTM: Table of Contents updated correctlyThe Table of Contents has been consistently updated with the new "tawk" prefix for all relevant method names. This change aligns well with the overall renaming strategy.
Line range hint
707-721: LGTM: tawkStart section updated correctlyThe
tawkStartsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly.Tools
LanguageTool
[uncategorized] ~872-~872: Possible missing comma found.
Context: ...etWindowType Returns the current widget type whether it’s inline or embed. `Composi...(AI_HYDRA_LEO_MISSING_COMMA)
726-746: LGTM: tawkShutdown section updated correctlyThe
tawkShutdownsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly.
751-776: LGTM: tawkMaximize section updated correctlyThe
tawkMaximizesection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkMaximizemethod.
781-806: LGTM: tawkMinimize section updated correctlyThe
tawkMinimizesection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkMinimizemethod.
811-836: LGTM: tawkToggle section updated correctlyThe
tawkTogglesection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkTogglemethod.
841-866: LGTM: tawkPopup section updated correctlyThe
tawkPopupsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkPopupmethod.
909-934: LGTM: tawkShowWidget section updated correctlyThe
tawkShowWidgetsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkShowWidgetmethod.
939-964: LGTM: tawkHideWidget section updated correctlyThe
tawkHideWidgetsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkHideWidgetmethod.
969-994: LGTM: tawkToggleVisibility section updated correctlyThe
tawkToggleVisibilitysection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkToggleVisibilitymethod.
Line range hint
999-1036: LGTM: tawkGetStatus section updated correctlyThe
tawkGetStatussection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkGetStatusmethod.
Line range hint
1041-1070: LGTM: tawkIsChatMaximized section updated correctlyThe
tawkIsChatMaximizedsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkIsChatMaximizedmethod.
Line range hint
1075-1104: LGTM: tawkIsChatMinimized section updated correctlyThe
tawkIsChatMinimizedsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkIsChatMinimizedmethod.
Line range hint
1109-1138: LGTM: tawkIsChatHidden section updated correctlyThe
tawkIsChatHiddensection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkIsChatHiddenmethod.
Line range hint
1143-1172: LGTM: tawkIsChatOngoing section updated correctlyThe
tawkIsChatOngoingsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkIsChatOngoingmethod.
Line range hint
1177-1206: LGTM: tawkIsVisitorEngaged section updated correctlyThe
tawkIsVisitorEngagedsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly. The usage oftawkOnLoadin the examples is appropriate and demonstrates the correct way to use thetawkIsVisitorEngagedmethod.
Line range hint
1211-1234: LGTM: tawkOnLoaded section updated correctlyThe
tawkOnLoadedsection has been properly updated with the new method name. The code examples for both Composition API and Options API reflect this change correctly.
| inject : ['tawkOnLoad'], | ||
|
|
||
| mounted() { | ||
| this.onLoad(() => { | ||
| this.tawkOnLoad(() => { |
There was a problem hiding this comment.
Update Composition API example and preceding text for consistency.
While the Options API example has been updated correctly, the Composition API example and the text preceding both examples still reference the old onLoad function name. To maintain consistency across the documentation:
- Update the Composition API example to use
tawkOnLoad. - Modify the text preceding the code examples to mention
tawkOnLoadinstead ofonLoad.
Here's a suggested diff for the Composition API example:
<script setup>
import { inject } from 'vue';
- const onLoad = inject('onLoad');
+ const tawkOnLoad = inject('tawkOnLoad');
- onLoad(() => {
+ tawkOnLoad(() => {
// place your function here
});
</script>Also, update the preceding text to reflect this change:
-You can listen on events emitted by the plugin by Injecting the function using `inject()`,
-It must be exactly match the name used to listen to that event, you can see the list of
+You can listen to events emitted by the plugin by injecting the function using `inject()`.
+It must exactly match the name used to listen to that event (with the 'tawk' prefix). You can see the list of
[events](api-reference.md) here.Committable suggestion was skipped due to low confidence.
Summary
There are users reported collision of provider keys since we use generic naming on our API. I added prefix
tawkin all actions, getters, listeners, and setters. This will resolve the issue #6Summary by CodeRabbit
These updates ensure a more intuitive experience when interacting with the TawkMessenger functionalities.