More CSS updates to Autocomplete
#2000
Conversation
馃 Changeset detectedLatest commit: 865cd5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -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.
This was originally changed to inline-block to remediate this bug we saw in Firefox:
However, inline-block results in the listbox overflowing out of its container. See primer/view_components#1050 (comment). We found the overlap issue does not appear when testing the swapped instances in prod in FireFox. Though this bug surfaces in Primer CSS docs, we don't see it when listbox is hidden THEN appears, so I think we should move forward with this to remediate the overflow issue.
| .input-group-button--autocomplete-embedded-icon { | ||
| vertical-align: bottom; | ||
| } | ||
|
|
There was a problem hiding this comment.
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.
| .form-control.autocomplete-embedded-icon-wrap { | ||
| &:focus-within { | ||
| background-color: var(--color-canvas-default); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
When an input in a .form-group is focused, it changes colors. We need to make sure that color of the parent embedded icon container also changes.
This fixes how the color is not uniform in this screenshot : https://github.com/github/github/pull/214426#discussion_r835613107
| </style> | ||
| ``` | ||
|
|
||
| ### Inline |
There was a problem hiding this comment.
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.
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?
src/forms/input-group.scss
Outdated
| // Autocomplete with embedded icon | ||
| .form-control.autocomplete-embedded-icon-wrap { | ||
| display: inline-flex; | ||
| padding: $spacer-1*1.25 $spacer-2; |
There was a problem hiding this comment.
Can you also make this change for the other autocomplete form controls? $spacer-1 * 1.25
I feel like it needs a space in the math but maybe it doesn't matter 馃
What are you trying to accomplish?
When updating Autocomplete PVC API and making swaps in dotcom, we saw some funkiness, particularly in the context of Form groups and Input groups.
This PR addresses the bugs we saw. Further details are in the comments.
What should reviewers focus on?
Is there a better approach? The CSS feels kinda brittle as need to support these various cases.
Definitely open to suggestions!
Are additional changes needed?