chore(stage-ui,stage-web,stage-tamagotchi): add ph-no-capture to chat messages#833
Conversation
Summary of ChangesHello @shinohara-rin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
⏳ Approval required for deploying to Cloudflare Workers (Preview) for stage-web.
Hey, @nekomeowww, @sumimakito, @luoling8192, @LemonNekoGH, kindly take some time to review and approve this deployment when you are available. Thank you! 🙏 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the ph-no-capture class to prevent capturing chat messages, which is a good privacy improvement. My review focuses on improving consistency and maintainability. I've suggested consolidating multiple class attributes into a single :class binding in ChatAssistantItem.vue and ChatUserItem.vue for better code clarity. I also noted an inconsistency in how ph-no-capture is applied in InteractiveArea.vue compared to ChatArea.vue, recommending a more robust approach by applying the class to a root container.
| <BasicTextarea | ||
| v-model="messageInput" | ||
| :placeholder="t('stage.message')" | ||
| class="ph-no-capture" |
There was a problem hiding this comment.
While adding ph-no-capture here works, it's inconsistent with the approach in ChatArea.vue where the class is applied to the root container. Applying ph-no-capture to the root div of this component (on line 162) would be a more robust and consistent solution, as it would cover all child elements, including the chat history and this textarea, with a single declaration. This would also make the ph-no-capture class on child components like ChatUserItem and ChatAssistantItem redundant within this context, but it's good they have it for standalone usage.
|
|
||
| <template> | ||
| <div flex :class="containerClass"> | ||
| <div flex :class="containerClass" class="ph-no-capture"> |
There was a problem hiding this comment.
To improve code clarity and maintainability, it's better to combine the static class with the dynamic :class binding into a single array. This avoids having multiple sources for an element's classes.
<div flex :class="[containerClass, 'ph-no-capture']">
|
|
||
| <template> | ||
| <div v-if="message.role === 'user'" :class="containerClasses"> | ||
| <div v-if="message.role === 'user'" :class="containerClasses" class="ph-no-capture"> |
There was a problem hiding this comment.
To improve code clarity and maintainability, it's better to combine the static class with the dynamic :class binding into a single array. This avoids having multiple sources for an element's classes.
<div v-if="message.role === 'user'" :class="[containerClasses, 'ph-no-capture']">
2 participants
No description provided.