🇵🇹Portugal @bbruno

I want to add a note here, that the above is also touched with the changes to make the Clear history work with the new Toolbar implementation, seen here: https://www.drupal.org/project/ai/issues/3548397 ✨ The toolbar chatbot needs to be able to clear threads Active

I have updated both the existing sticky and toolbar chats so that they are both able to clear their histories, with a global Drupal.clearDeepchatMessages coming from deepchat-init.js. This to prevent having to recreate the same logic across any further extra deepchat implementations.

I have kept the styling minimal here although we can revisit it at any time, but for now this feature is available again.

Example:

I can replicate the issue and can confirm that the MR above fixes the issue.

Before

Now

I am also able to replicate the error message in the description when attempting to clear history.

I can confirm checking out the MR above fixes the issue. Code looks good, only small nitpick is that `let connect` can be changed to `const connect`, since this is not reassigned at any point. But everything is working as expected.

@christophweber the proposed solution at the time of issue creation only took into account keeping track of user messages and AI's response to provide a better context of the conversation to the AI, it did not include the more advanced history tracking mechanism that we have now in the AI module with structured output and tool calling. Unfortunately it'll need to be assessed again with these newer requirements if we're looking to provide the same functionality.

Related issue to the comment above regarding the Chat expansion: https://www.drupal.org/project/ai/issues/3518356 ✨ Extend current ChatBot overlay (modal) functionality Active

Existing bottom position options have been re-added along with their styles and functionality. There should now be no breaking change. New blocks will come with the new configuration by default, aka 'Toolbar' position/style.

Back up for review

Assigning back to me to add support for the existing Bottom positionings. The current solution was taking into account completely replacing these, but after discussing we will keep the Bottom positions for now, along with the new sidebar option.

Current Status

Following up from last week's meeting where we got a pretty clear idea on how to move forward, we have created a small demo with the following suggestions that align best with Gin's ideas moving forward, and also provides a similar experience to Experience Builder in terms of design for a very nice consistency across both:

• - Do not place any AI related functionality on the side navigation itself, this will eventually be 'hidden' in Gin, which we do not want since we want to really show off the AI button and functionalities
• - Place the AI Chatbot button in the top left of the toolbar, this will be the designated spot for Tools in Gin and also follows what XB is also doing
• - Have the Chatbot itself 'slide' out from the left of the screen, pushing the rest of the page to the right (Not overlayed). This allows us to not have to occupy any existing real-estate within menus or sections of the page, and also not have to overlay any content - This was important as in the future it is planned for the chatbot to provide real times changes occurring on the page which should be visible to the user. There were concerns if this will impact performance since we would be moving content and admin menus, but this so far seems to not be the case.

There was also the suggestion that the chatbot should remember its opened state, this to ensure consistency in the case that the Chatbot takes you elsewhere, or you are navigating around the site with the Chatbot already opened

Here is the current look of the demo presented in the AI alignment meeting with the changes above taken into account. I have also attached a video with the opening and closing animations and message design:

However, there are still some more things left to discuss:

How should this best be shown on mobile? The most popular idea in the alignment meeting was for the AI Chatbot to take up the entire screen when opened, but instead of sliding from the left, to be draggable from bottom/top. This allows for an easy hiding/showing of the rest of the content on the page.

How should we best label the Assistant button? This will also impact how users will refer to the AI assistant itself. Currently it is just 'Assistant', maybe that is fine.

MR LGTM. I've tested this on Gin Light, Gin Dark, Claro and it all looks as expected. Even the highlighting color when hovering over the button is working. Nice!

Thank you for the information!

Current status

The current wireframes had the idea to introduce the chatbot as a button in the side admin navigation, which when opened pushed down the rest of the menu items and allowed for the chatbot to exist on the page without having to take up any extra real estate on the screen (which also did not cover the content on the page).

However, we have heard from the Gin team that moving forward, the navigation bar is not planned to be around for very long, or rather mostly hidden with emphasis on the top bar for tools and actions - which breaks the idea above as the chatbot needs to always be standing out and not tucked away with the rest of the navigation.

As for now, as also suggested by the Gin team, it is best if the AI Chatbot button lives somewhere in the top bar on the far left side, similar to the current implementation in Experience Builder, to provide the most similar feeling. This would be somewhere where currently the breadcrumbs are present, or as in the wireframe, the "Take a tour" button, although it will mostly be a space for tools in the future.
As for the chatbot placement itself, this is still to be tried out and discussed, but once again staying similar to the XB experience, the top candidate is having the chatbot grow from the left of the screen - pushing the entire page to the right. Although there are some concerns if this is not too heavy on browser resources.

Uploading a .patch here from the MR since this has not been merged yet.

+1 This is working nicely for me, and has fixed the issue. Thanks!

It would be nice to get some maintainer information first, perhaps this is not the intended behaviour and removing is not what we want. Although it does seem to be the case for now :)

bbruno → created an issue.

I could replicate the issue, and can confirm that checking out this MR also fixes the layout issue on my side.

However, the streaming functionality itself does not seem to be working for me. Unsure if this is something wrong with local configuration or an existing problem. Using gpt-4o model.

I was also having this issue, checked out the MR and this fixes it!

kristen pol → credited bbruno → .

kristen pol → credited bbruno → .

bbruno → made their first commit to this issue’s fork.

bbruno → changed the visibility of the branch 3532137-avoid-relying-on to hidden.

bbruno → changed the visibility of the branch 3532137-avoid-relying-on to active.

Changes from https://www.drupal.org/project/ai/issues/3532137 📌 Avoid relying on LLM for markup in the content suggestions modal Active have been merged onto this issues MR to prevent having to redo work, since the changes would conflict with each other if either was merged first, and would cause unnecessary updates.

Since this was created as a follow up from https://www.drupal.org/project/ai/issues/3531588 ✨ Per-field ai_content_suggestions UX improvements Active , and both MRs will contain conflicts and adjustments that would be needed if either one is merged first, we'll include the changes from here, there. This way we dont have to make the same changes twice.

I agree, merging either of these solutions first will cause an unnecessary adjustment right after. If we really want to go ahead with changing the prompt fully, doesnt make much sense to alter it here imo. +1

I've removed the now unnecessary event listeners that were added earlier in the MR since we are using buttons, also added the update hook. Up for review :)

bbruno → created an issue.

Using a button instead would indeed be the preferable choice, but I think then it would also require some extra styling to prevent the native 'button' look. At least now using the Gin or Claro admin themes, it looks a bit out of place.

If we were to update the prompt, I believe we'd also need a new update hook instead of adjusting the current one thats there?

Overall I think the method of using markup thats returned by the LLM in the suggestions modal needs to be rethought. Relying on a model to return the correct markup has not been the most efficient in some cases we've seen, also when tried on a 'dumber' model. Makes certain tasks like this perhaps harder than necessary. I will create an issue about this.

I have added the possibility to navigate the dialog and select the suggestions available using keyboard to the existing MR. Back up for review

The button is now accessible and we can open the suggestions dialog, however the dialog itself is still not navigable by keyboard since the LLM returns the markup using ``'s. I would assume we also want this to be working, setting back to needs work :)

I've taken a look at the JS related changes, from my side everything looks good. Very interesting how you've made this work with the deep-chat framework! Looks great.

Up for review! Would be great if someone can take a look :)

Fixed MR merge conflicts

@sirclickalot Thanks for the extra context and quick reply! I just took a look and yes, when inserting a paragraph within a paragraph this uses '.button--small' and it happens because it comes from the Gin template.

I was only checking from my side when adding the 'first level' paragraphs, which use a hardcoded class added at FrontendEditingController.php.

Could you confirm if the MR I provided above fixes this issue?

This has been fixed, the SVG did indeed contain a built in shadow, it was also not correctly sized and included an empty space around the image itself which made it not possible to add a shadow from the CSS side - this was also fixed. The shadow is now applied using CSS and can be overwritten with CSS up to the user's liking

Up for review

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to hidden.

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to active.

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to hidden.

I can replicate this, working on it

bbruno → made their first commit to this issue’s fork.

Hello @sirclickalot, you are right in saying that this CSS you are encountering does not seem to have a use. More even so since from previous commits we are no longer using the '.button--small' class on this paragraph list, but '.button--large'.

I have provided an MR that completely removes this CSS, as I mentioned does not have a use. However, I am still intrigued as to why your environment still contains these small buttons, considering that the class was changed in the same merge request as the CSS above.

I invite you to take a look and suggest any other options if this is still not as expected. Thank you!

bbruno → made their first commit to this issue’s fork.

bbruno → made their first commit to this issue’s fork.

We have worked on this at 1xINTERNET and would want to share our work

We have already implemented this at 1xINTERNET and would want to contribute our work

bbruno → created an issue.

bbruno → created an issue.

Added a proposed solution to the issue as mentioned above we have worked on this at 1xINTERNET and would contribute our work

It seems that this happened when the ai_ckeditor module was initially ported over, would be nice if someone can take a look here. Thanks!

bbruno → created an issue.

This is a nice idea, I have tested it locally and it works as expected. I agree with mrdalesmith that it perhaps is a little non-standard, but in my opinion provides a much better ux than the current option.

Non-related to the issue itself, but the changes in the package-lock.json and yarn.lock indicate that we have multiple package handlers at work here, it would be best to stick to one to avoid any inconsistencies due to unsynced lock files.

bbruno → created an issue.

Created the MR above which contains a check for if the body element does not contain the class .frontend-editing--hidden before applying the related styling, as is the case for the rest of the hidden styles. Up for review :)

bbruno → created an issue.

Hi sirclickalot, the SVGs that are used in the actions links are actually background images applied to pseudo-element '::before' on each link.

Since these are black, they can be easily manipulated using a 'Filter' style.

For example, this filter when targeting this '::before', will turn the icons red:

.frontend-editing__action::before { 
 filter: invert(20%) sepia(90%) saturate(4516%) hue-rotate(357deg) brightness(97%) contrast(135%);
}

You can search online for 'CSS filter black to hex' and find tools that will provide you with the correct filter styling to match your desired color

Added the changes into the MR above.

This removes the 'bottom' positioning and instead replaces it with the 'fixed' positioning, which is now applied whenever the element's top goes out of view. It also means we can get rid of the 'bottom' observer, since there is no longer a change in positioning depending on if the bottom of an element becomes visible.

Up for review.

This adjustments make sense, I will start taking a look here.

breidert → credited bbruno → .

MR updated, please proceed Artem.

Updated MR !73 to use the IntersectionObserver API as an alternative to the Scroll event listener due to performance.

Back to Needs Review

Rerolling the last patch as it is not working with the latest recommended version. It had been very useful as the photoswipe images do not come by default with any lazy-loading attributes and there are no settings to do so.

Thanks SirClickalot for that last comment, it helped with reaching a possible solution since we can not replicate your issue. Updated the MR with a fix that should hopefully sort this out. Up for review.

The new custom color variable has been implemented on the toggle. Up for review.

Bug where the hover mode padding was still being applied although the toggle is set to 'Off' has been fixed. Putting up for review.

bbruno → changed the visibility of the branch 3439916-introduce-color-customization to active.

bbruno → changed the visibility of the branch 3439916-introduce-color-customization to hidden.