Conversation
Summary of ChangesHello @Adib234, 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 refactors the banner display logic to enable more precise tracking of how often specific messages are shown to users. Instead of a single counter for a default banner, the system now maintains a map of banner messages to their individual display counts, allowing for better control and analytics over banner visibility. 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
|
There was a problem hiding this comment.
Code Review
This pull request refactors the banner display logic to track view counts for each unique banner message, instead of a single global count. This is a great improvement for more granular control over banner visibility.
However, I've identified a critical issue in the implementation within AppHeader.tsx. The component's state for bannerCounts is not updated after being incremented, causing the banner to be displayed indefinitely regardless of the view count. I've provided a detailed comment and a code suggestion to fix this.
Additionally, a test case in packages/cli/src/ui/components/AppHeader.test.tsx seems to have been missed during the refactoring. The test 'should not render the default banner if shown count is 5 or more' (line 129) still uses the old data structure for mocking persistentState.get(), which will cause the test to fail. This should be updated to align with the new bannerCounts object structure.
|
Ok |
|
Size Change: +956 B (0%) Total Size: 21.1 MB ℹ️ View Unchanged
|
b11bda3 to
1785bd4
Compare
…essages instead of times banner has been displayed (google-gemini#13428)
… banner has been displayed (google-gemini#13428)
|
/patch stable |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
… banner has been displayed (#13428) # Conflicts: # packages/cli/src/ui/components/AppHeader.tsx
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
… banner has been displayed (#13428)
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
… banner has been displayed (#13428)
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
… banner has been displayed (google-gemini#13428)
Summary
We would like to keep track of counts by the number of times a message has shown and not just how many times the banner has been shown.
Details
Related Issues
How to Validate
Manually validated by changing default text on debug settings.
Pre-Merge Checklist