-
Notifications
You must be signed in to change notification settings - Fork 1.3k
More CSS updates to Autocomplete
#2000
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
Changes from all commits
56dbb05
9a30bad
6759a54
9719cf2
ca8872b
40901d4
0e7d60b
4b81386
865cd5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@primer/css": patch | ||
--- | ||
|
||
More CSS updates to `Autocomplete` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ | |
// Wrapper for the input and result elements to ensure alignment | ||
.autocomplete-body { | ||
position: relative; | ||
display: inline-block; | ||
display: inline; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was originally changed to However, |
||
} | ||
|
||
// Wrapper and conditional styles for when an icon is added | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,13 @@ | |
// stylelint-disable-next-line primer/spacing | ||
margin: 15px 0; | ||
|
||
// Autocomplete with embedded icon | ||
.form-control.autocomplete-embedded-icon-wrap { | ||
&:focus-within { | ||
background-color: var(--color-canvas-default); | ||
} | ||
} | ||
|
||
Comment on lines
+18
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When an input in a This fixes how the color is not uniform in this screenshot : https://github.com/github/github/pull/214426#discussion_r835613107 |
||
// Text fields | ||
.form-control { | ||
width: 440px; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,12 @@ | |
&.inline { | ||
display: inline-table; | ||
} | ||
|
||
// Autocomplete with embedded icon | ||
.form-control.autocomplete-embedded-icon-wrap { | ||
display: inline-flex; | ||
padding: $spacer-1 * 1.25 $spacer-2; | ||
} | ||
} | ||
|
||
.input-group .form-control, | ||
|
@@ -30,6 +36,10 @@ | |
vertical-align: middle; // Match the inputs | ||
} | ||
|
||
.input-group-button--autocomplete-embedded-icon { | ||
vertical-align: bottom; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addresses: https://github.com/github/github/pull/214426/files#r836499311. The changes in this file addresses an issue where button positioning is way off when autocomplete with embedded icon is inside an input group. |
||
.input-group .form-control:first-child, | ||
.input-group-button:first-child .btn { | ||
border-top-right-radius: 0; | ||
|
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.
It appears Inline label doesn't work within
Input group
. (Example of inline label within input group)Any suggestions for making this work?
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.
I wonder if there are any examples of this usage in dotcom? If not, maybe we should just not advertise that this is a combination. If people end up needing it later on, we could address then. Thoughts?
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.
I agree!