Skip to content

Conversation

@glenn-jocher
Copy link
Member

@glenn-jocher glenn-jocher commented Nov 23, 2025

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Improves the resilience of the Ultralytics Chat widget in SPA environments by marking key elements as permanent and hardening DOM reattachment logic.

📊 Key Changes

  • ➕ Added a markPermanent helper that sets data-turbo-permanent and data-turbolinks-permanent on critical chat elements.
  • 🔁 Updated watchForRemoval to:
    • Use lazy parent accessors (() => document.head/body) instead of static references.
    • Observe document.documentElement with subtree: true for more robust detection across SPA navigations.
    • Immediately re-attach tracked elements if they are missing, and now include the global tooltip element in the watch list.
  • 🧩 Applied markPermanent to the injected style tag, backdrop, pill, modal, and tooltip in both chat.js and the minified chat.min.js bundle.

🎯 Purpose & Impact

  • 🛡️ Reduces the chance that SPA or Turbo/Turbolinks navigations will remove or duplicate the chat widget’s core DOM nodes.
  • 🚀 Provides smoother, more stable behavior when navigating between pages or views, especially in Turbo/Turbolinks-powered apps.
  • 🔧 Low-risk structural change limited to DOM lifecycle handling and attributes, with no impact on public APIs or UX flows beyond improved reliability.

@vercel
Copy link

vercel bot commented Nov 23, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
llm Ready Ready Preview Comment Nov 23, 2025 4:14pm

@UltralyticsAssistant
Copy link
Member

👋 Hello @glenn-jocher, thank you for submitting a ultralytics/llm 🚀 PR! This is an automated review assistant, and a Ultralytics engineer will be with you shortly. In the meantime, please review the checklist below to ensure a smooth merge process:

  • Define a Purpose: Clearly explain the purpose of your fix or feature in your PR description, and link to any relevant issues. Ensure your commit messages are clear, concise, and adhere to the project's conventions.
  • Synchronize with Source: Confirm your PR is synchronized with the ultralytics/llm main branch. If it's behind, update it by clicking the 'Update branch' button or by running git pull and git merge main locally.
  • Ensure CI Checks Pass: Verify all Ultralytics Continuous Integration (CI) checks are passing. If any checks fail, please address the issues.
  • Update Documentation: Update the relevant documentation for any new or modified features.
  • Add Tests: If applicable, include or update tests to cover your changes, and confirm that all tests are passing.
  • Sign the CLA: Please ensure you have signed our Contributor License Agreement if this is your first Ultralytics PR by writing "I have read the CLA Document and I sign the CLA" in a new message.
  • Minimize Changes: Limit your changes to the minimum necessary for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." — Bruce Lee

For more guidance, please refer to our Contributing Guide. If you have any questions or need clarification, feel free to leave a comment in this PR. Thank you for contributing to Ultralytics! 🚀

Copy link
Member

@UltralyticsAssistant UltralyticsAssistant left a comment

Choose a reason for hiding this comment

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

🔍 PR Review

Made with ❤️ by Ultralytics Actions

Key SPA-hardening goals aren’t met yet: permanent flags need stable IDs to take effect, and the new document-wide MutationObserver runs on every mutation, which can noticeably hurt performance. Address these to ensure the widget stays stable without regressing page responsiveness.

💬 Posted 2 inline comments

📋 Skipped 1 file (lock files, minified, images, etc.)
  • js/chat.min.js

@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixed Bug has been resolved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants