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

Check for wf-loading class to be in the html element before injecting #28

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

marcosvrs
Copy link
Owner

As a workaround on the current issue where the injection of the WPP library throws an error if the WhatsApp web app isn't completely loaded, we wait until the class wf-loading is added to the html element so we know the web app is loaded, and the WPP can be injected safely.
This should work around the issue #27 while we don't have a final solution from wppconnect.

@marcosvrs marcosvrs force-pushed the bugfix/27_Loader-injecting-failure branch from 96945fd to b350831 Compare December 8, 2023 09:58
@marcosvrs
Copy link
Owner Author

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

PR Analysis

  • 🎯 Main theme: This PR is a workaround for an issue where the injection of the WPP library throws an error if the WhatsApp web app isn't completely loaded. The solution is to wait until the class wf-loading is added to the html element, indicating that the web app is loaded, before injecting the WPP library.
  • 📝 PR summary: The PR modifies the wa-js.ts file to check for the presence of the wf-loading class in the html element before injecting the WPP library. It also introduces a MutationObserver to watch for changes in the html element's class attribute. Additionally, it modifies the manifest.json file to include wppconnect.js in the list of web accessible resources.
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 4, because the PR involves changes in the core functionality of the application and requires a deep understanding of the WPP library and the MutationObserver API.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR addresses a critical issue and provides a workaround solution. However, it would be beneficial to include error handling for the import statement and the observer. Also, it would be better to encapsulate the logic of checking the wf-loading class and injecting the WPP library into a separate function for better readability and maintainability.

  • 🤖 Code feedback:

How to use

Instructions

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

@marcosvrs marcosvrs merged commit 16add5c into master Dec 9, 2023
2 checks passed
@marcosvrs marcosvrs deleted the bugfix/27_Loader-injecting-failure branch December 9, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants