Skip to content

Conversation

bangonicdd2
Copy link
Contributor

@bangonicdd2 bangonicdd2 commented Sep 21, 2025

PR Checklist

  • Have you checked if it works normally in all models? Ignore this if it doesn't use models.
  • Have you checked if it works normally in all web, local, and node hosted versions? If it doesn't, have you blocked it in those versions?
  • Have you added type definitions?

Description

Currently, when using HTML elements in background embedding slots, inline styles can be used, but direct CSS classes(ie. x-risu-something) cannot be specified. This change resolves this issue, but if you don't want this or have other ideas, please let me know.

(+@)
Thanks to comments, I noticed button triggers are not available in background embedding, so I added capturing of button triggers here as well. For performance reasons, chat reload is left at the trigger's discretion.

@tammy3211
Copy link
Contributor

I suggest adding background embedding support for the risu-trigger attribute.

@bangonicdd2
Copy link
Contributor Author

I suggest adding background embedding support for the risu-trigger attribute.

Could you elaborate a little more?

@tammy3211
Copy link
Contributor

It is not possible to activate V2 triggers or Lua script triggers using buttons or other elements created in background embedding. I wish this could be made possible.

@bangonicdd2 bangonicdd2 marked this pull request as draft September 26, 2025 21:25
@bangonicdd2 bangonicdd2 changed the title fix: apply 'chattext' class on backgroundEmbedding feat: css class & button trigger support on background DOM Sep 27, 2025
@bangonicdd2 bangonicdd2 marked this pull request as ready for review September 27, 2025 06:14
@tammy3211
Copy link
Contributor

Sorry for another suggestion, but I noticed that when the chattext attribute is applied to background embedding, it can sometimes interfere with regex scripts.
(This issue already existed before, but it was not visible on the screen. Now it shows up visually, so users can notice it.)
Maybe we could consider adding a custom flag option that controls whether regex scripts are applied to background embedding (default off)?

@bangonicdd2
Copy link
Contributor Author

bangonicdd2 commented Sep 27, 2025

Sorry for another suggestion, but I noticed that when the chattext attribute is applied to background embedding, it can sometimes interfere with regex scripts. (This issue already existed before, but it was not visible on the screen. Now it shows up visually, so users can notice it.) Maybe we could consider adding a custom flag option that controls whether regex scripts are applied to background embedding (default off)?

I feel cautious about PR that change existing behavior like that. Are there chatbots or cases I can test that might interfere with this change?

@tammy3211
Copy link
Contributor

Sorry for another suggestion, but I noticed that when the chattext attribute is applied to background embedding, it can sometimes interfere with regex scripts. (This issue already existed before, but it was not visible on the screen. Now it shows up visually, so users can notice it.) Maybe we could consider adding a custom flag option that controls whether regex scripts are applied to background embedding (default off)?

I feel cautious about PR that change existing behavior like that. Are there chatbots or cases I can test that might interfere with this change?

I did a few more experiments and found that this usually happens when regex is set to capture everything with flags like Dot All (s) (which also captures newline \n). This is often used to insert specific phrases or HTML at the end or the beginning of the chat.
However, I also discovered that the background embedding transformation issue can be avoided by leveraging the fact that {{chat_index}} in cbs outputs -1 when used with background embedding.
So I don’t think we actually need to add a custom flag option for this. Sorry for the confusion earlier.

@bangonicdd2
Copy link
Contributor Author

Actually separating out the background embedding area as new option from the existing display regex seems a neat idea, but implementing it in this PR seems out of scope.

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