-
Notifications
You must be signed in to change notification settings - Fork 280
feat: make summary thread similar to normal thread style #11575
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout changes look great! Only change for now is to use the filled version of the assistant icon :)
Looks really nice! Couple of points:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newest screenshot looks really nice! only 3 small notes:
- I see that in the German screenshot the "Summary is AI generated" notice is still in english, it should also be translated to whatever language
- The strings seem to wrap and ellipsize a lot because they're all too long in German :( Maybe we could do "Newest message" instead of "Go to newest message"?
- The assistant variables are available now just as an FYI nextcloud/server#54679 they should be used as much as possible in case they are changed later on closer to the release.
thats because its on my local env, and its not pushed yet to transifex :) |
c4cede6
to
39faf01
Compare
@GretaD are the three items of #11575 (review) done? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
img/creation-gradient.svg
seems unused?
yes, the first point is not relevant. The other 2 are done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! Only 2 small changes:
- In the thread summary, the assistant icon should be center aligned with the avatar in the messages below (seems like it's a bit to the right in the screenshot)
- In the AI replies, we can remove the icon (the gradient outline is already very eye catching) and make the text color --main-text (I'm concerned about a11y of the gradient on unbolded text)
Other than that it's good to to! Approving to not block :)
I can change the colour of the text, but removing the icon means changing the component that we just created for the assistantbuttons: https://nextcloud-vue-components.netlify.app/#/Components/Nextcloud%20Assistant?id=ncassistantbutton I mean, i can remove it just for mail with :deep(), but that means that the assistant buttons will be different in different part of the software. |
Oh hmm, in that case pinging @marcoambrosini to double check the usage of the button |
I think it is fine to use Text only buttons too |
comment from Marco: Nimisha might be out for today, so I'd suggest to keep it as you have it for now and then we can always hack with a quick pr, or update the components to create more variants |
66936f5
to
5a6674b
Compare
Signed-off-by: greta <[email protected]>
5a6674b
to
997c19e
Compare
/backport to stable5.4 |
ref #11328
smart reply:

thread summary:

Envelope summary:
