feat: add cursor pointer on selector element #3346
Conversation
anmolyadav-dev
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg and
magicmatatjahu
as code owners
WalkthroughThe pull request introduces a new CSS rule in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3346 +/- ##
=======================================
Coverage 59.01% 59.01%
=======================================
Files 21 21
Lines 649 649
=======================================
Hits 383 383
Misses 266 266 ☔ View full report in Codecov by Sentry. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3346--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/languageSelector/LanguageSelect.tsx (1)
19-19: LGTM! The cursor-pointer implementation meets the requirements.
The addition of cursor-pointer class successfully addresses the UX improvement requested in issue #3345.
Consider extracting the base classes into a constant for better maintainability:
+ const baseClasses = 'form-select h-full py-0 px-3 pr-7 inline-flex justify-center rounded-md border border-gray-300 shadow-sm py-2 bg-white text-sm font-medium text-gray-700 hover:bg-gray-50 focus:border-gray-500 focus:outline-none focus:ring-0 focus:ring-black cursor-pointer';
return (
<select
data-testid='Select-form'
onChange={(ev) => onChange(ev.target.value)}
- className={twMerge(
- `form-select h-full py-0 px-3 pr-7 inline-flex justify-center rounded-md border border-gray-300 shadow-sm py-2 bg-white text-sm font-medium text-gray-700 hover:bg-gray-50 focus:border-gray-500 focus:outline-none focus:ring-0 focus:ring-black ${className} cursor-pointer`
- )}
+ className={twMerge(baseClasses, className)}
value={selected}
>
|
You can try to Fix the PR title and raise a PR again to get through the lintPR title |
anshgoyalevil
changed the title
asyncapi-bot
requested review from
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
|
@anmolyadav-dev That's a nice addition. Example: one such dropdown is on page https://www.asyncapi.com/tools/generator |
|
@anshgoyalevil Sure i will search and fix more of these . thankyou for the feedback. |
|
@anshgoyalevil i changed the global style for select element so every select element should now have a cursor pointer |
anshgoyalevil
removed request for
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
anshgoyalevil
left a comment
anshgoyalevil
left a comment
There was a problem hiding this comment.
Thanks @anmolyadav-dev 🚢
anshgoyalevil
changed the title
|
@anmolyadav-dev |
|
@anshgoyalevil |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
styles/globals.css (1)
355-357: LGTM! Consider these enhancements for better UX.While the implementation addresses the core requirement, consider these improvements:
- Add hover state for more precise control
- Add transition for smoother interaction
- Fix formatting (add space after
cursor:)-select { - cursor:pointer; -} +select { + cursor: default; + transition: cursor 0.2s ease; +} + +select:hover { + cursor: pointer; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
styles/globals.css(1 hunks)
🔇 Additional comments (1)
styles/globals.css (1)
355-357: Verify impact on other select elements.
Since this is a global style, let's verify that it doesn't adversely affect other select elements in the codebase.
|
/rtm |
|
@anshgoyalevil |
That shouldn't happen. What are the changes introduced in those files? |
|
@anshgoyalevil no visible changes. it just shows file changed and add it in status
|
4 participants
Description
added cursor pointer over language selector
Related issue(s)
Fixes #3345
Summary by CodeRabbit
selectelement to indicate interactivity, enhancing the user experience.