-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/renaming api name #40
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes involve a comprehensive renaming of function callbacks and properties related to the Tawk API across multiple files. All previous naming conventions have been prefixed with "tawk" to enhance consistency. This includes updates to method signatures, prop names, and documentation to reflect the new naming structure, ensuring that all references to Tawk-related methods and properties are uniform. The overall functionality and logic of the API and components remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TawkMessenger
participant App
User->>App: Interacts with App
App->>TawkMessenger: Calls tawkOnLoad
TawkMessenger->>User: Loads chat widget
User->>TawkMessenger: Requests to minimize
TawkMessenger->>App: Calls tawkMinimize
App->>TawkMessenger: Updates widget state
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (5)
src/index.js (2)
65-161
: LGTM! Consistent renaming of API methods.The changes in this section are consistent with the PR objectives. All API methods have been correctly prefixed with "tawk", improving the naming convention and reducing the likelihood of collisions.
Consider adding a brief comment at the beginning of the
useImperativeHandle
hook to explain the purpose of the "tawk" prefix, which could help future maintainers understand the reasoning behind this naming convention.
301-320
: LGTM! Consistent renaming of prop types.The changes in the
propTypes
object are consistent with the PR objectives. All callback prop types have been correctly prefixed with "tawk", maintaining consistency with the earlier changes.Consider updating the JSDoc comments for these props (if they exist) to reflect the new naming convention. This will ensure that the documentation remains in sync with the code changes.
docs/api-reference.md (3)
Line range hint
64-1237
: Method renaming implemented consistently.All methods have been correctly renamed with the "tawk" prefix in both the headings and code examples. The functionality descriptions remain accurate, and the examples have been updated to reflect the new naming convention.
For improved consistency, consider updating the text descriptions to include the "tawk" prefix when referring to method names. For example, in the
tawkOnLoad
section, you could update the description to:"Callback function invoked right after the widget is rendered. This callback is not supported in pop out chat window. The
tawkOnLoad
function can be used to..."This change would make the documentation even more consistent and clear for users.
Tools
LanguageTool
[style] ~403-~403: 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)
Line range hint
1241-1257
: Update secureMode section for consistency.The secureMode section has not been updated to reflect the new naming convention. To maintain consistency with the rest of the API, please make the following changes:
- Update the method name in the example code:
-tawkMessengerRef.current.visitor({ +tawkMessengerRef.current.tawkVisitor({ name : 'name', email : '[email protected]' hash : '<calculate-hash>' });
- Consider updating the section title to "tawkSecureMode" if this is intended to be a method name.
These changes will ensure that the secureMode section aligns with the new naming convention used throughout the rest of the API documentation.
Tools
LanguageTool
[style] ~403-~403: 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)
402-415
: Use gender-neutral language in documentation.To make the documentation more inclusive, consider updating the description for the
tawkOnVisitorNameChanged
function to use gender-neutral language.Change:
"Callback function invoked when the visitor manually changes his name."To:
"Callback function invoked when the visitor manually changes their name."This small change makes the documentation more inclusive and aligns with best practices for technical writing.
Tools
LanguageTool
[style] ~403-~403: 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)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/api-reference.md (37 hunks)
- docs/how-to-use.md (3 hunks)
- src/index.js (4 hunks)
Additional context used
LanguageTool
docs/api-reference.md
[style] ~403-~403: 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)
Additional comments not posted (9)
docs/how-to-use.md (4)
16-17
: LGTM! Consistent renaming applied.The changes to the function name
handleTawkMinimize
and the method calltawkMinimize()
are consistent with the PR objective of adding the "tawk" prefix to API components. This improves clarity and reduces the likelihood of naming collisions.
22-22
: LGTM! Consistent update of event handler.The
onClick
handler has been correctly updated to use the new function namehandleTawkMinimize
, maintaining consistency with the earlier renaming.
27-27
: Correct usage of ref prop.The change from
useRef
toref
is correct.useRef
is the React hook used to create the ref, whileref
is the proper prop name for passing the ref to a component. This change improves the accuracy of the example.
Line range hint
1-50
: Overall, documentation updates align well with PR objectives.The changes in this file successfully implement the "tawk" prefix for API components in the documentation and code examples. This aligns well with the PR objective of addressing naming collisions and improving clarity. The examples have been updated to reflect the new naming conventions for functions, methods, and props.
A few key points:
- Function names and method calls have been consistently updated (e.g.,
handleTawkMinimize
,tawkMinimize()
).- Component props have been renamed to include the "tawk" prefix (e.g.,
tawkOnLoad
).- The
useRef
prop correction toref
improves the accuracy of the React usage example.These changes should help users understand and implement the updated API correctly, reducing the likelihood of naming conflicts in their applications.
src/index.js (2)
261-280
: LGTM! Consistent renaming of default props.The changes in the
defaultProps
object are consistent with the PR objectives. All callback prop names have been correctly prefixed with "tawk", maintaining consistency with the earlier changes.
Line range hint
1-324
: Overall assessment: PR objectives met with minor suggestions for improvementThe changes in this file successfully address the PR objectives by consistently prefixing all relevant API methods, props, and callbacks with "tawk". This should help reduce naming collisions as intended.
Key points:
- API methods, props, and callbacks have been consistently renamed.
- The overall functionality of the component remains unchanged.
- The code structure and quality have been maintained.
Suggestions for improvement:
- Consider adding a comment explaining the "tawk" prefix in the
useImperativeHandle
hook.- Review and potentially update the event names in
addEventListener
calls for consistency.- Update JSDoc comments (if they exist) to reflect the new naming convention.
These changes significantly improve the API's clarity and should reduce the likelihood of naming conflicts. Great job on maintaining consistency throughout the file!
docs/api-reference.md (3)
12-60
: Table of contents updated correctly.The table of contents has been consistently updated with the "tawk" prefix for all method names. The structure and order remain unchanged, which maintains clarity for users.
Line range hint
1259-1390
: Clarify intention for customstyle object naming.The customstyle section and its related properties (zIndex, visibility) have not been updated with the "tawk" prefix. It's unclear whether this is intentional or if it was overlooked during the renaming process.
If the customstyle object is meant to be part of the API's public interface, consider whether it should be renamed to maintain consistency with the rest of the API (e.g.,
tawkCustomStyle
). If it's intended to remain as is, please confirm that this is the desired behavior.Could you please clarify the intended naming convention for the customstyle object and its properties?
Tools
LanguageTool
[style] ~403-~403: 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)
Line range hint
1-1390
: Overall assessment: Renaming implemented successfully with minor improvements needed.The renaming of API methods and callbacks by adding the "tawk" prefix has been largely successful and consistent throughout the documentation. The changes improve the API's namespace and should help prevent potential naming conflicts for users.
To finalize this update, please address the following points:
- Update the secureMode section to use the new
tawkVisitor
method name.- Clarify the naming convention for the customstyle object and its properties.
- Use gender-neutral language in the
tawkOnVisitorNameChanged
function description.- Consider updating text descriptions to include the "tawk" prefix when referring to method names for improved consistency.
Once these minor issues are addressed, the documentation will be fully consistent and ready for release.
Tools
LanguageTool
[style] ~403-~403: 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)
@@ -35,7 +35,7 @@ Using the API callbacks, pass a function as props on the callback you will used. | |||
|
|||
```js | |||
function App() { | |||
const onLoad = () => { | |||
const tawkOnLoad = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Consistent renaming with a minor inconsistency.
The changes to the function name tawkOnLoad
and the corresponding prop tawkOnLoad
in the TawkMessengerReact component are consistent with the PR objective of adding the "tawk" prefix to API components. This improves clarity and reduces the likelihood of naming collisions.
However, there's a small inconsistency in line 47:
- tawkOnLoad={onLoad}/>
+ tawkOnLoad={tawkOnLoad}/>
The prop value should reference the new function name tawkOnLoad
instead of onLoad
.
Also applies to: 47-47
props.tawkOnLoad(); | ||
}); | ||
|
||
window.addEventListener('tawkStatusChange', (status) => { | ||
props.onStatusChange(status.detail); | ||
props.tawkOnStatusChange(status.detail); | ||
}); | ||
|
||
window.addEventListener('tawkBeforeLoad', () => { | ||
props.onBeforeLoad(); | ||
props.tawkOnBeforeLoad(); | ||
}); | ||
|
||
window.addEventListener('tawkChatMaximized', () => { | ||
props.onChatMaximized(); | ||
props.tawkOnChatMaximized(); | ||
}); | ||
|
||
window.addEventListener('tawkChatMinimized', () => { | ||
props.onChatMinimized(); | ||
props.tawkOnChatMinimized(); | ||
}); | ||
|
||
window.addEventListener('tawkChatHidden', () => { | ||
props.onChatHidden(); | ||
props.tawkOnChatHidden(); | ||
}); | ||
|
||
window.addEventListener('tawkChatStarted', () => { | ||
props.onChatStarted(); | ||
props.tawkOnChatStarted(); | ||
}); | ||
|
||
window.addEventListener('tawkChatEnded', () => { | ||
props.onChatEnded(); | ||
props.tawkOnChatEnded(); | ||
}); | ||
|
||
window.addEventListener('tawkPrechatSubmit', (data) => { | ||
props.onPrechatSubmit(data.detail); | ||
props.tawkOnPrechatSubmit(data.detail); | ||
}); | ||
|
||
window.addEventListener('tawkOfflineSubmit', (data) => { | ||
props.onOfflineSubmit(data.detail); | ||
props.tawkOnOfflineSubmit(data.detail); | ||
}); | ||
|
||
window.addEventListener('tawkChatMessageVisitor', (message) => { | ||
props.onChatMessageVisitor(message.detail); | ||
props.tawkOnChatMessageVisitor(message.detail); | ||
}); | ||
|
||
window.addEventListener('tawkChatMessageAgent', (message) => { | ||
props.onChatMessageAgent(message.detail); | ||
props.tawkOnChatMessageAgent(message.detail); | ||
}); | ||
|
||
window.addEventListener('tawkChatMessageSystem', (message) => { | ||
props.onChatMessageSystem(message.detail); | ||
props.tawkOnChatMessageSystem(message.detail); | ||
}); | ||
|
||
window.addEventListener('tawkAgentJoinChat', (data) => { | ||
props.onAgentJoinChat(data.detail); | ||
props.tawkOnAgentJoinChat(data.detail); | ||
}); | ||
|
||
window.addEventListener('tawkAgentLeaveChat', (data) => { | ||
props.onAgentLeaveChat(data.detail); | ||
props.tawkOnAgentLeaveChat(data.detail); | ||
}); | ||
|
||
window.addEventListener('tawkChatSatisfaction', (satisfaction) => { | ||
props.onChatSatisfaction(satisfaction.detail); | ||
props.tawkOnChatSatisfaction(satisfaction.detail); | ||
}); | ||
|
||
window.addEventListener('tawkVisitorNameChanged', (visitorName) => { | ||
props.onVisitorNameChanged(visitorName.detail); | ||
props.tawkOnVisitorNameChanged(visitorName.detail); | ||
}); | ||
|
||
window.addEventListener('tawkFileUpload', (link) => { | ||
props.onFileUpload(link.detail); | ||
props.tawkOnFileUpload(link.detail); | ||
}); | ||
|
||
window.addEventListener('tawkTagsUpdated', (data) => { | ||
props.onTagsUpdated(data.detail); | ||
props.tawkOnTagsUpdated(data.detail); | ||
}); | ||
|
||
window.addEventListener('tawkUnreadCountChanged', (data) => { | ||
props.onUnreadCountChanged(data.detail); | ||
props.tawkOnUnreadCountChanged(data.detail); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider updating event names for consistency.
While the callback prop names have been correctly prefixed with "tawk", the event names in addEventListener
calls remain unchanged (e.g., 'tawkLoad', 'tawkStatusChange', etc.). This might lead to inconsistency between the event names and the new callback prop names.
Consider updating the event names to match the new naming convention. For example:
-window.addEventListener('tawkLoad', () => {
+window.addEventListener('tawkTawkLoad', () => {
props.tawkOnLoad();
});
Alternatively, if the event names are part of an external API that cannot be changed, please add a comment explaining why they remain unchanged while the callback names have been updated.
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 tawk in all actions, getters, listeners, and setters.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation