Conversation
…at/about-dialog-overhaul
Summary of ChangesHello, 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 a significant overhaul of the application's 'About' dialog, transforming it into a more informative and aesthetically pleasing interface. The changes aim to provide users with a clearer overview of application details, acknowledge project contributors dynamically, and offer direct access to community resources. This modernization enhances the overall user experience and streamlines the maintenance of project information. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a completely redesigned "About" dialog, moving from a simple table to a more modern, card-based layout. It includes new icons, a list of contributors fetched from a JSON file, and links to GitHub and donation pages. A script to automatically update the contributors list from the GitHub API is also added.
The code is well-structured, but I've found a few areas for improvement. Specifically, there's a logic issue in a useCallback hook that causes unnecessary data fetching, a missing key prop in a React list which can cause rendering issues, and an opportunity to improve type safety in the new contributor update script. My detailed comments are below.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…at/about-dialog-overhaul
…at/about-dialog-overhaul
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the About dialog to include detailed contributor information, project history, and dynamic icon switching. It introduces new UI components such as FluidWrapper and PropertySheet, enhances the Modal component for mobile responsiveness, and adds a script to automate contributor list updates. Technical feedback identifies several areas for improvement: optimizing the onLoad callback's dependency array to prevent redundant network requests, resolving inconsistent React and Preact imports, implementing pagination for the GitHub contributors API, defining missing TypeScript types in the update script, and addressing potential memory leaks and closure issues in the hover state management logic.
| } | ||
|
|
||
| async function fetchContributors() { | ||
| const response = await fetch("https://api.github.com/repos/TriliumNext/Trilium/contributors"); |
There was a problem hiding this comment.
The GitHub API for contributors returns a maximum of 30 results per page by default. For a project of this scale, it is likely that there are more than 30 contributors. Consider adding ?per_page=100 to the URL or implementing pagination to ensure the list is complete.
| const response = await fetch("https://api.github.com/repos/TriliumNext/Trilium/contributors"); | |
| const response = await fetch("https://api.github.com/repos/TriliumNext/Trilium/contributors?per_page=100"); |
| } | ||
| } | ||
|
|
||
| function processContributorList(contributorInfo: GithubContributor[]) { |
| const createContributorHoverHandler = useCallback(() => { | ||
| let timeoutID; | ||
| return (contributor: Contributor, isHovering: boolean, part: "name" | "role") => { | ||
| if (part === "role" && contributor.role === "original-dev") { | ||
| if (isHovering) { | ||
| timeoutID = setTimeout(() => { | ||
| setAltIcon("classic"); | ||
| }, 500); | ||
| } else { | ||
| clearTimeout(timeoutID); | ||
| setAltIcon(null); | ||
| } | ||
| } | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
The timeoutID variable is shared across all items in the contributor list because it's defined in the closure of the handler creator. While currently only one contributor has the original-dev role, this logic is fragile. Furthermore, the timeout is not cleared if the component unmounts or the dialog closes, which could attempt to update state on an unmounted component. Consider using a ref or a more robust way to manage per-item hover states and cleanups.
… list (local Git + GitHub listing)
No description provided.