-
Notifications
You must be signed in to change notification settings - Fork 86
added tooltip bubble for project name #175
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Vedansh Saini <[email protected]>
Reviewer's GuideInserted an inline tooltip bubble next to the βYour Project Nameβ header by wrapping a question-mark icon and explanatory text in a tooltip container. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @vedansh-5 - I've reviewed your changes - here's some feedback:
- Consider extracting the tooltip markup into a reusable component or template so you can easily apply it to other fields without duplicating HTML.
- Enhance accessibility by adding ARIA attributes (e.g., role="tooltip" and aria-describedby) and keyboard support for the tooltip bubble.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the tooltip markup into a reusable component or template so you can easily apply it to other fields without duplicating HTML.
- Enhance accessibility by adding ARIA attributes (e.g., role="tooltip" and aria-describedby) and keyboard support for the tooltip bubble.
## Individual Comments
### Comment 1
<location> `src/popup.html` </location>
<code_context>
+ <h4>Your Project Name<span class="tooltip-container ml-2">
</code_context>
<issue_to_address>
Consider improving tooltip accessibility for screen readers.
Use ARIA attributes like aria-label or aria-describedby, and ensure tooltip content is accessible via keyboard focus to support screen reader users.
</issue_to_address>
Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
<h4>Your Project Name</h4> | ||
<h4>Your Project Name<span class="tooltip-container ml-2"> | ||
<i class="fa fa-question-circle question-icon"></i> | ||
<span class="tooltip-bubble"> |
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.
suggestion: Consider improving tooltip accessibility for screen readers.
Use ARIA attributes like aria-label or aria-describedby, and ensure tooltip content is accessible via keyboard focus to support screen reader users.
Signed-off-by: Vedansh Saini <[email protected]>
@hpdang I have added a bubble for users to know what project name does. Does this PR make sense? Thankyou. |
π Fixes
Fixes #154
π Summary of Changes
πΈ Screenshots / Demo (if UI-related)
β Checklist
π Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Enhancements: