Skip to content
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

SelectPanel2: Only show clear action in search with text #4146

Merged
merged 7 commits into from
Jan 23, 2024

Conversation

siddharthkp
Copy link
Member

@siddharthkp siddharthkp commented Jan 13, 2024

Clear action is only visible when there is something to clear

selectpanel-clear-action.mov

@siddharthkp siddharthkp self-assigned this Jan 13, 2024

This comment was marked as resolved.

Copy link
Contributor

github-actions bot commented Jan 13, 2024

size-limit report 📦

Path Size
dist/browser.esm.js 104.47 KB (0%)
dist/browser.umd.js 105.11 KB (0%)

// '& input:empty + .TextInput-action': {display: 'none'},
}
}
sx={{'&:has(input:placeholder-shown) .TextInput-action': {display: 'none'}}}
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this was fun! was able to use a new css feature and a really old css feature

@siddharthkp siddharthkp marked this pull request as ready for review January 22, 2024 18:33
@siddharthkp siddharthkp requested a review from a team January 22, 2024 18:33
Copy link
Member

@broccolinisoup broccolinisoup left a comment

Choose a reason for hiding this comment

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

Looks great!

One tiny comment: I know this is nitpicking but I just want to mention. I don't think placeholder is an exposed prop in the API but funny thing is that when the placeholder='' instead of placeholder='Search', the css logic works as expected in Chrome but not in Safari. In Safari, when the placeholder is an empty string, the clear action is visible. I found out while I was like 'Ohh placeholder-shown, let me play with this' 😄

@siddharthkp
Copy link
Member Author

siddharthkp commented Jan 23, 2024

the css logic works as expected in Chrome but not in Safari.

Interesting, this is what I see in Arc. I imagine it's because there is no placeholder to show, so it's now shown 😅

no placeholder, action is shown

I want to ignore this use case, at least for dotcom, because there should always be a placeholder. Maybe we can add a type error in TextInput for empty placeholder

@siddharthkp siddharthkp added this pull request to the merge queue Jan 23, 2024
Merged via the queue into main with commit 4e7404c Jan 23, 2024
30 checks passed
@siddharthkp siddharthkp deleted the drafts-selectpanel-clear-action branch January 23, 2024 10:34
@primer primer bot mentioned this pull request Jan 23, 2024
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.

2 participants