fix: resolve skills form UI issues#305
Conversation
|
@abhi-nav-25 is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Hi 👋 I’d like to work on this issue under GSSoC 2026. I reviewed the recommendation engine requirements and plan to improve the scoring logic while keeping the implementation modular and maintainable. I’ll test the changes locally before submitting the PR. Please assign this issue to me. Thanks! |
|
hi @komalharshita please check my PR |
|
@abhi-nav-25 kindly resolve the merge conflicts |
|
@komalharshita Plz check my PR |
komalharshita
left a comment
There was a problem hiding this comment.
Thanks for the contribution. Since the skills form is now a heavily shared/core interaction component in DevPath, this PR needs deeper validation before it can be safely merged.
Recent PRs have already modified:
- skills overflow handling
- chip rendering
- placeholder behavior
- responsive layout
- helper text/accessibility
Because of this, even small UI adjustments in this section now carry elevated regression risk.
At the moment, the PR does not provide enough validation evidence that the issue is fully resolved across all interaction states.
Before approval, please provide:
- clear before/after reproduction screenshots
- mobile and desktop validation
- testing for long skill names and multiple wrapped chips
- validation for rapid add/remove interactions
- confirmation that no overflow/alignment regressions were introduced
The branch should also be manually tested locally before merge to verify:
- chip wrapping behavior
- responsive layout stability
- keyboard/focus interactions
- compatibility with previously merged skills UI fixes
Once these validations are completed and no regressions are found, the PR can be re-reviewed.
|
@komalharshita Validation completed after syncing branch with latest upstream/main. Tested:
Results:
|
komalharshita
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier validation requests and re-testing the skills form against recent upstream changes. The tooltip behavior is improved and the regression testing effort is appreciated.
There are still a few issues that should be resolved before merge:
- Please remove the added
<br><br>spacing and replace it with proper CSS-based spacing/margins. Layout spacing should not be handled with manual line breaks. - Tooltip accessibility is still incomplete. The current implementation only works on hover. Please add keyboard/focus support (
:focus-within) and improve accessibility semantics (aria-label/aria-describedby). - The tooltip width is hardcoded to
260px, which may still overflow on smaller mobile screens. Consider using responsive sizing (max-width,clamp, etc.). - Some useful tooltip affordance styling from the previous implementation (
cursor: help, centered icon behavior) was removed and should be refined rather than dropped entirely.
Good progress overall, but these accessibility and maintainability concerns should be addressed before approval.
|
Hi @komalharshita Changes made: Validation performed: Thanks for the detailed review. Looking forward to any further feedback |
|
Hi @komalharshita, Please check my PR |
|
Hi @komalharshita, |
komalharshita
left a comment
There was a problem hiding this comment.
Thanks for the updates and for addressing much of the earlier feedback.
The tooltip implementation is significantly improved, and I appreciate the effort put into accessibility, responsive sizing, and validation testing.
However, I cannot approve this PR yet for the following reasons:
• Multiple tooltip instances use the same ID (skills-tooltip). IDs must be unique across the page.
• All tooltip buttons reference the same aria-describedby target, creating invalid accessibility relationships.
• The PR still contains additional spacing/layout modifications that extend beyond the original issue scope and increase regression risk in a shared component.
• The requested :focus-within accessibility support was not added.
• The branch currently has merge conflicts that must be resolved before review can continue.
Please address these issues and update the branch for another review.
Thank you for the contribution.
a2fd2d3 to
8845e08
Compare
|
@komalharshita Please check now!! |
|
Approved for merge |
Summary [required]
This PR fixes the tooltip UI issue in the Skills Form section. Previously, hovering over the info icon only displayed a question mark instead of the intended tooltip content. The tooltip now correctly displays helper text on hover, and spacing/styling inconsistencies were also improved for a cleaner UI experience.
Related Issue [required]
Closes #237
Type of Change [required]
Bug fix — resolves a broken behaviour
Style — CSS or visual changes only, no logic change
What Was Changed [required]
File ------- | Change made -- |index.html | Updated tooltip structure and fixed hover behaviour
style.css -- | Improved tooltip styling, spacing, and visibility
Self-Review Checklist [required]
I have read CONTRIBUTING.md and followed all guidelines
My branch name follows the convention:
feat/,fix/,docs/,data/,style/,test/I have tested the changes locally
I have not introduced any
print()orconsole.log()debug statementsI have not modified files outside the scope of the linked issue
If I changed the UI, I tested it at 375px (mobile) and 1280px (desktop)
Notes for Reviewer
Please verify tooltip visibility and spacing consistency across different screen sizes.