- Issue created by @Leo Pitt
- Merge request !694Make AI Suggestions button visible at all times. Normalize width of enabled fields. → (Open) created by Leo Pitt
- 🇬🇧United Kingdom Leo Pitt
I have changed this issue to focus just on the accessibility issue with the "AI Suggestions" button. I suggest that we create a separate issue to consider the placement of the "AI Suggestions" button.
The button now visible at all times so that it can be accessed by the keyboard. The "AI Function" psuedo-element has been removed.
Please review.
- 🇵🇹Portugal bbruno Poland
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 :)
- 🇵🇹Portugal bbruno Poland
I have added the possibility to navigate the dialog and select the suggestions available using keyboard to the existing MR. Back up for review
- 🇬🇧United Kingdom Leo Pitt
Thanks for this. Agreed that the suggestions also need to be accessible.
I haven't tried this out, but I am wandering why we cannot change the `` to a `` in the prompt?
E.g. So that the prompt reads:
You will be asked to help with content suggestions based on the input in markdown format. Answer with suggestions only, do not repeat the task description, do not use any other text except for the suggestions. Wrap each suggestion in a button tag that has the class "ai-cs-use". Here is an example of a single suggestion: ```html <span class="ai-content-suggestion"><button class="ai-use-cs" aria-label="Use this suggestion">Suggestion</button></span> ``` If there are multiple suggestions, ensure that these are wrapped in <li> tags inside an ordered list <ol>. Here is an example of multiple suggestions: ```html <ol> <li> <span class="ai-content-suggestion"> <button class="ai-use-cs" aria-label="Use this suggestion">Suggestion</button> </span> </li> <li> <span class="ai-content-suggestion"> <button class="ai-use-cs">Another suggestion</button> </span> </li> </ol> ``` If there is too little information to generate a response, respond with "Insufficient information provided, please add more data before generating suggestions."
Would this mean that we no longer need to use JS to set the suggestion tab index and role?
- 🇬🇧United Kingdom Leo Pitt
I have updated this to use button elements in the returned suggestions, since that feels like the semantically appropriate element, and also solves accessibility issues.
Please review.
- 🇵🇹Portugal bbruno Poland
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.
- 🇬🇧United Kingdom Leo Pitt
I think I agree with what you are saying bbruno.
Perhaps let's add some styling to the button and add an update hook.
I strongly agree regarding using the LLM to return markup - ideally the LLM should return responses in a more consistent format e.g. json and we would output the results in appropriate markup outside of that.
- 🇬🇧United Kingdom Leo Pitt
Updated button styes to fit in better with the modal.
Todo: Update hook.
- 🇵🇹Portugal bbruno Poland
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 :)
- 🇩🇪Germany a.dmitriiev
Maybe we should really merge this issue with 📌 Avoid relying on LLM for markup in the content suggestions modal Active as we are changing the same things now and even creating the update hooks.
- 🇵🇹Portugal bbruno Poland
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
- 🇵🇹Portugal bbruno Poland
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.
- 🇩🇪Germany a.dmitriiev
I have rebased MR because of this issue ✨ Add processor and field widget assist plugin system to AI Content Suggestions Active