Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix/renaming api name #40

wants to merge 2 commits into from

Conversation

jaoaustero
Copy link
Collaborator

@jaoaustero jaoaustero commented Sep 25, 2024

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

    • Enhanced consistency in the Tawk API naming conventions with new prefixes for function callbacks and properties.
    • Updated documentation to reflect changes in naming conventions for better clarity.
  • Bug Fixes

    • Corrected prop names in the TawkMessengerReact component for improved functionality.
  • Documentation

    • Revised API reference and usage guides to align with the new naming structure.

@jaoaustero jaoaustero added the enhancement New feature or request label Sep 25, 2024
@jaoaustero jaoaustero self-assigned this Sep 25, 2024
Copy link

coderabbitai bot commented Sep 25, 2024

Walkthrough

The 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

Files Change Summary
docs/api-reference.md Renamed function callbacks and properties by prefixing with "tawk"; updated documentation accordingly.
docs/how-to-use.md Renamed handleMinimize to handleTawkMinimize, updated onLoad to tawkOnLoad; modified props.
src/index.js Renamed methods and props in TawkMessenger component by prefixing with "tawk"; no functional 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
Loading

Poem

🐇 In the meadow, changes bloom,
Tawk's new names dispel the gloom.
Functions dance with a prefix bright,
Consistency shines, a joyful sight.
With every hop, we celebrate,
Tawk's new path, oh, isn't it great! 🌼


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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:

  1. Update the method name in the example code:
-tawkMessengerRef.current.visitor({
+tawkMessengerRef.current.tawkVisitor({
    name : 'name',
    email : '[email protected]'
    hash : '<calculate-hash>'
});
  1. 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

Commits

Files that changed from the base of the PR and between 7ba0a3d and b484296.

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 call tawkMinimize() 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 name handleTawkMinimize, maintaining consistency with the earlier renaming.


27-27: Correct usage of ref prop.

The change from useRef to ref is correct. useRef is the React hook used to create the ref, while ref 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:

  1. Function names and method calls have been consistently updated (e.g., handleTawkMinimize, tawkMinimize()).
  2. Component props have been renamed to include the "tawk" prefix (e.g., tawkOnLoad).
  3. The useRef prop correction to ref 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 improvement

The 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:

  1. API methods, props, and callbacks have been consistently renamed.
  2. The overall functionality of the component remains unchanged.
  3. The code structure and quality have been maintained.

Suggestions for improvement:

  1. Consider adding a comment explaining the "tawk" prefix in the useImperativeHandle hook.
  2. Review and potentially update the event names in addEventListener calls for consistency.
  3. 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:

  1. Update the secureMode section to use the new tawkVisitor method name.
  2. Clarify the naming convention for the customstyle object and its properties.
  3. Use gender-neutral language in the tawkOnVisitorNameChanged function description.
  4. 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 = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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

Comment on lines +172 to +248
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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant