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/chat: Wait for 'ready' before sending theme data. #2371

Merged

Conversation

dominiccooney
Copy link
Contributor

@dominiccooney dominiccooney commented Oct 2, 2024

We used to inject themes using CSSOM injected by the client. We moved this code to the Agent to share it with VS, etc. and used DOMContentLoaded as the signal to do it. This races with webview React, which may be responding to this signal by DOMContentLoaded, but is not guaranteed to be.

Now we watch for the webview to send the 'ready' event before dispatching the initial theme update message.

Fixes CODY-3796, BUGS-126, BUGS-233, BUGS-154.

Test plan

Manually tested:

  • Start and restart the IDE a bunch of times, the theme should be applied.
  • Switch the theme, that should still work.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @dominiccooney and the rest of your teammates on Graphite Graphite

@dominiccooney dominiccooney marked this pull request as ready for review October 2, 2024 10:41
@dominiccooney
Copy link
Contributor Author

We have a ./gradlew check failure, this fails for me locally with and without this commit. Maybe main is flaky or broken? The commit history tells the tale.

Here's the failure I'm getting locally:

AllSuites > com.sourcegraph.cody.edit.DocumentCodeTest.testGetsWorkingGroupLens FAILED
    com.intellij.testFramework.TestLoggerFactory$TestLoggerAssertionError at TestLoggerFactory.java:403
        Caused by: java.lang.Throwable at Logger.java:370

AllSuites > com.sourcegraph.cody.edit.DocumentCodeTest.testUndo FAILED
    java.lang.NoClassDefFoundError at DaemonListeners.java:343
        Caused by: java.lang.NoClassDefFoundError at DaemonListeners.java:343
            Caused by: java.lang.ExceptionInInitializerError at TestLoggerFactory.java:403

AllSuites > com.sourcegraph.cody.edit.DocumentCodeTest.testShowsAcceptLens FAILED
    java.lang.NoClassDefFoundError at DaemonListeners.java:343
        Caused by: java.lang.NoClassDefFoundError at DaemonListeners.java:343
            Caused by: java.lang.ExceptionInInitializerError at TestLoggerFactory.java:403

AllSuites > com.sourcegraph.cody.edit.DocumentCodeTest.testAccept FAILED
    java.lang.NoClassDefFoundError at DaemonListeners.java:343
        Caused by: java.lang.NoClassDefFoundError at DaemonListeners.java:343
            Caused by: java.lang.ExceptionInInitializerError at TestLoggerFactory.java:403

4 tests completed, 4 failed

Copy link
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

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

LGTM

@dominiccooney dominiccooney enabled auto-merge (squash) October 2, 2024 10:59
@dominiccooney dominiccooney merged commit 2e3082f into main Oct 2, 2024
13 of 14 checks passed
@dominiccooney dominiccooney deleted the 10-02-fix_chat_wait_for_ready_before_sending_theme_data branch October 2, 2024 12:26
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