Skip to content

finished inputs #8

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

finished inputs #8

wants to merge 8 commits into from

Conversation

Chaitya2108
Copy link
Contributor

@Chaitya2108 Chaitya2108 commented Apr 14, 2025

pull request for inputs, making dropdowns in a separate branch

image

Copy link

Thanks for contributing!
It looks like you haven't updated this package's version in package.json.
Please update the package version according to semantic versioning guidelines.

Copy link
Member

@SheepTester SheepTester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trying it out, it looks good

I think we should allow passing through more HTML props on <input> (see below), plus a few nitpicks

When it shows a hint, I can't click on the text itself to edit it. Maybe pointer-events: none it?
Though that said, on mobile there's currently no way to accept the hint. Maybe tapping on the hint should accept it, while tapping anywhere else goes through to the input. Apparently a child element can override a parent's pointer-events: none so maybe this isn't too difficult to do

Copy link

github-actions bot commented May 5, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://acmucsd.github.io/components/pr-preview/pr-8/

Built to branch gh-pages at 2025-05-05 22:46 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dropdown looks too tall

image

maybe missing box-sizing: border-box?

}

return (
<div className={`dropdown-wrapper ${theme} ${size}`}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably also have a <select> element so that you can tab focus on it, mostly for accessibility, but on keyboard you can also type the first few letters to select an option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants