input element shows repeating background on hover when disabled is true

[kimberlythegeek]

Description

input element shows repeating background image of the search icon when disabled is set

Steps to reproduce

go to https://mozilla.github.io/perfcompare/
edit HTML of search input element, add disabled
hover over search bar

Expected result

would expect search bar to look normal except gray text and icon

Actual result

repeating background of search icon

Environment

firefox release, ubuntu 18

[craigcook]

Looks like this is coming from this line: https://github.com/mozilla/protocol/blob/main/src/assets/sass/protocol/base/elements/_forms.scss#L218

input[type='search']:disabled has the same specificity as input[type='search']:hover but the hover style comes later in the cascade so it's overriding the previous styling, and without the other rules for background-position, background-repeat, etc. I think the best fix is to increase specificity, perhaps with an explicit rule for for :disabled:hover. We could also try using not(:disabled) but that's not supported in older browsers.

[avi5079]

Hey @craigcook , @kimberlythegeek So I am working on this issue and I think I have fixed the issue but I don't know by changing scss file on protocol how it will reflect the change on perfcompare repo.
Could you please guide me for that?

[craigcook]

Feel free to submit a pull request with your fix. Once a fix is merged we'll need to publish a new version of Protocol to NPM, and the perf comparison site can then update their dependency to get the latest CSS.

[avi5079]

Hey @craigcook , So I added this code in the _forms.scss file in input[type='search'] selector, but it is showing error in the terminal, can you guide me how to fix this error?

    &:disabled {
        background-image: $url-image-search-form;
        background-repeat: no-repeat;
        background-position: left 8px top 50%;
        padding: $field-padding $field-padding $field-padding
            calc(1.5em + #{$field-padding * 2});
        &:hover {
            background-image: $url-image-search-form;
        }
    }

Terminal error :

[craigcook]

Sorry for the slow response.

The bidi mixin on line 402 requires a comma after the first close parenthesis:

@include bidi(((padding, 0 $spacing-sm 0 0, 0 0 0 $spacing-sm),)); <-- note the comma in ),))

This is because it's designed to accept a comma-separated list but isn't quite smart enough to know the difference, so it always expects a comma even when there's only one item. We could probably adjust the mixin to be a bit more accepting but for now it's pretty strict on how its input must be formatted (we also need to improve our documentation of some mixins because none of this is obvious or discoverable, sorry about that).

[avi5079]

Hey @craigcook, Thank you for the explanation. So, I checked the perfCompare webpage and now it is not showing the search bar. So, Can you guide me how can I test my changes and should I submit a pull request or not.

[craigcook]

The simplest way to test Protocol styles locally is on the Protocol website itself. You can see a search field in http://localhost:3000/patterns/atoms/forms.html#other-input-types though it's not disabled. You can temporarily add a disabled attribute in the template at https://github.com/mozilla/protocol/blob/main/src/patterns/atoms/forms/other-input.hbs#L17 for testing, just remember not to include that in your commit.