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

Display AI panel in code mode #2076

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

Conversation

jurgenwerk
Copy link
Contributor

@jurgenwerk jurgenwerk commented Jan 22, 2025

This PR is the first step towards adding AI assistant functionality in code mode (previously it was present in interact mode only). There will be more work to make AI more helpful in the code mode context which will be covered in separate PRs (e.g. adding fields to card definitions, card auto attachment behaviour,...)

There are now 4 columns in case the AI panel is open:
image

‼️ Monaco code editor theming problem

Since there can now be multiple, differently themed Monaco code editors shown on the same page, we have run into a well known issue (microsoft/monaco-editor#338) where monaco editor does not support differently themed editors on the same page:

image

The reason is that the monaco editor is storing its CSS in a global <style> tag and every new instance rendered will override that CSS with the the newly rendered instance's theme.

Here is an example of the style tag:
monaco-style-tag.txt

I tried to manipulate this tag's contents by namespacing these selectors (by rewriting them) and have 2 style tags that have all these selectors namespaced for bright and dark monaco theme but the solution seemed too hacky, it resulted in editor flashing, and the repaints needed after manipulating style tags probably caused performance degradations.

Workaround with CSS filter

One of the suggestions in microsoft/monaco-editor#338 was to use a filter to achieve a dark theme to simplify the solution so I went with that. So this means using this piece of code on the white themed editor to achieve the dark themed editor:

:global(.preview-code .monaco-editor) {
  filter: invert(1) hue-rotate(151deg) brightness(0.8) grayscale(0.1);
}

Potential drawbacks

1️⃣ The dark theme is changed

Old theme (vs-dark theme):

image

New theme (standard vs theme + css filter):

image

We can of course tweak the colors by changing the hue-rotate param but I thought this value looks pretty good.

2️⃣ The dark theme could be unpredictable/inflexible for future requirements

Since the dark theme is a function of the white theme, we can't easily define individual colors in dark theme (can only tweak the css filter).

Copy link

github-actions bot commented Jan 22, 2025

Host Test Results

    1 files  ±0      1 suites  ±0   21m 52s ⏱️ +9s
734 tests +1  732 ✔️ +1  2 💤 ±0  0 ±0 
739 runs  +1  737 ✔️ +1  2 💤 ±0  0 ±0 

Results for commit 47c35a1. ± Comparison against base commit 673efbc.

♻️ This comment has been updated with latest results.

This fixed the "You attempted to get the value of a helper after the helper was destroyed, which is not allowed" error in "Commands tests: a command sent via SendAiAssistantMessageCommand without autoExecute flag is not automatically executed by the bot"
@jurgenwerk jurgenwerk marked this pull request as ready for review January 22, 2025 12:41
@jurgenwerk jurgenwerk requested a review from a team January 22, 2025 12:41
Copy link
Contributor

@FadhlanR FadhlanR left a comment

Choose a reason for hiding this comment

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

I tested it in the staging preview, and it works great!

@FadhlanR FadhlanR requested a review from a team January 22, 2025 13:16
@@ -250,6 +251,9 @@ export default class Room extends Component<Signature> {
messageElemements: new WeakMap(),
messageScrollers: new Map(),
messageVisibilityObserver: new IntersectionObserver((entries) => {
if (isDestroyed(this)) {
Copy link
Contributor Author

@jurgenwerk jurgenwerk Jan 22, 2025

Choose a reason for hiding this comment

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

I was getting the You attempted to get the value of a helper after the helper was destroyed, which is not allowed"
error in the Commands tests: a command sent via SendAiAssistantMessageCommand without autoExecute flag is not automatically executed by the bot test.

My guess is that when the AI panel is opened in interactive mode and a switch to code mode happens, the room gets rerendered (due to operator mode state change) and the intersection observer wants to work on the previous room rendering but it can't since it was destroyed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found a better approach: 47c35a1

Copy link
Contributor

@burieberry burieberry left a comment

Choose a reason for hiding this comment

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

Tested this locally, looks good. Nice workaround for the color theme issue.

Copy link
Contributor

@habdelra habdelra left a comment

Choose a reason for hiding this comment

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

I think the new dark theme looks great

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.

4 participants