Show inline image in replies; add icon to copy user handle #1088
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @alphatownsman, 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!
Summary of Changes
This pull request introduces two main enhancements. First, it enables the display of inline images within replies by modifying the replies.html template to handle image attachments differently, creating preview thumbnails and lightbox views. Second, it adds a copy icon next to the user handle in the sidebar, allowing users to easily copy the handle to their clipboard. The neodb-takahe subproject commit is also updated.
Highlights
- Inline Image Display: The
replies.htmltemplate is modified to display image attachments as inline thumbnails with a lightbox option for viewing the full image. Non-image attachments are still displayed as download links. - Copy User Handle: A copy icon is added next to the user handle in the sidebar, enabling users to copy the handle to the clipboard with a single click.
- Subproject Update: The
neodb-takahesubproject commit is updated to 90908b7.
Changelog
- common/templates/_sidebar.html
- Added a copy icon next to the user's handle.
- Implemented JavaScript to copy the handle to the clipboard when the icon is clicked.
- journal/templates/replies.html
- Modified the template to display image attachments as inline thumbnails.
- Added lightbox functionality to view full-size images when thumbnails are clicked.
- Updated the attachment display logic to differentiate between image and non-image attachments.
- Image attachments are now displayed as thumbnails with a lightbox, while other attachments are displayed as download links.
- neodb-takahe
- Updated subproject commit to 90908b700861f80a33d62fa6e4fae42f56cc5e74
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
A handle to copy with ease,
Inline images to appease,
A click, a view,
Enhancements new,
Code refined, if you please.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces two user-facing enhancements: displaying inline images in replies and adding a copy icon for user handles. The changes appear well-structured and improve the user experience. However, there are a few points that could be improved for better maintainability and clarity.
Summary of Findings
- Accessibility of Copy Handle Icon: The copy handle icon lacks proper accessibility features. Consider adding an
aria-labelto the icon to provide a descriptive text alternative for screen reader users. - Lightbox Implementation: The lightbox implementation could benefit from a more robust solution, potentially using a JavaScript library for better user experience and accessibility. The current CSS-only approach might have limitations in terms of keyboard navigation and focus management.
- Attachment Display Logic: The logic for displaying attachments is duplicated. Consider refactoring the attachment display logic into a reusable template or function to avoid redundancy.
Merge Readiness
The pull request is almost ready for merging. Addressing the accessibility concern for the copy handle icon is crucial. While the lightbox and attachment display logic improvements are not critical, they would significantly enhance the code's maintainability and user experience. I am unable to approve this pull request, and recommend that another reviewer does so after the identified issues have been addressed.
| @@ -40,6 +40,7 @@ | |||
| <h6 class="nickname">{{ identity.display_name }}</h6> | |||
| <div> | |||
| <span class="handler">@{{ identity.full_handle }}</span> | |||
| <span class="action inline"><span><small><a _="on click call navigator.clipboard.writeText('@{{ identity.full_handle }}') then halt"><i class="fa-regular fa-copy"></i></a></small></span></span> | |||
There was a problem hiding this comment.
Consider adding an aria-label attribute to the <i> tag to improve accessibility. This will provide a text alternative for screen reader users, describing the purpose of the icon. For example, aria-label="Copy user handle".
| <span class="action inline"><span><small><a _="on click call navigator.clipboard.writeText('@{{ identity.full_handle }}') then halt"><i class="fa-regular fa-copy"></i></a></small></span></span> | |
| <span class="action inline"><span><small><a _="on click call navigator.clipboard.writeText('@{{ identity.full_handle }}') then halt"><i class="fa-regular fa-copy" aria-label="Copy user handle"></i></a></small></span></span> |
| {% for attachment in post.attachments.all %} | ||
| {% if attachment.is_image %} | ||
| <a href="#img_{{ post.pk }}_{{ loop.index }}"> | ||
| <img src="{{ attachment.thumbnail_url.relative }}" | ||
| alt="image attachment" | ||
| class="preview"> | ||
| </a> | ||
| <a href="#" class="lightbox" id="img_{{ post.pk }}_{{ loop.index }}"> | ||
| <span style="background-image: url('{{ attachment.full_url.relative }}')"></span> | ||
| </a> | ||
| {% endif %} |
There was a problem hiding this comment.
The lightbox implementation is basic. Consider using a JavaScript library for a more feature-rich and accessible lightbox experience. This would allow for keyboard navigation, better focus management, and smoother transitions.
| {% for attachment in post.attachments.all %} | ||
| {% if attachment.is_image %} | ||
| <a href="#img_{{ post.pk }}_{{ loop.index }}"> | ||
| <img src="{{ attachment.thumbnail_url.relative }}" | ||
| alt="image attachment" | ||
| class="preview"> | ||
| </a> | ||
| <a href="#" class="lightbox" id="img_{{ post.pk }}_{{ loop.index }}"> | ||
| <span style="background-image: url('{{ attachment.full_url.relative }}')"></span> | ||
| </a> | ||
| {% endif %} | ||
| {% endfor %} | ||
| {% for attachment in post.attachments.all %} | ||
| {% if not attachment.is_image %} | ||
| <a href="{{ attachment.full_url.relative }}" class="download"> | ||
| <div> | ||
| <i class="fa-solid fa-download"></i> {{ attachment.file_display_name }} | ||
| </div> | ||
| </a> | ||
| {% endif %} | ||
| {% endfor %} |
There was a problem hiding this comment.
The attachment display logic is duplicated. The code iterates through post.attachments.all twice, once for images and once for other files. Consider refactoring this into a single loop within a reusable template or function to avoid redundancy and improve maintainability. You could pass a flag to the template to indicate whether to display images or other files.
No description provided.