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

fix(field): field borders now are correctly colored even on Safari #5737

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

Conversation

l0ll098
Copy link

@l0ll098 l0ll098 commented Oct 23, 2024

Hi there! This is a small patch to fix an annoying bug that occurs with Fields on Safari, both on macOS and iOS.

As also described in issue #5668, Fields such as <md-outlined-text-field> and <md-outlined-select>, when focused, show an underlying dark line, even if the primary theme color is set to something else. This patch addresses said issue.

Here you can take a look at what is going on with the latest version of this library (v2.2.0 at this moment).

If you look carefully at the focused input in the first screenshot, you'll see that dark line just beneath the label, whereas all other parts of that input are brown-ish.
The second image shows the resulting page after having applied this patch. Here you can see how the dark line is no longer visible and all parts of the input border have the same color.

Before patch After being patched

Even though these screenshots were captured on a iOS 18 Simulator, I've seen this issue both on real iOS devices, and previous versions of iOS Simulators. The same issue also occurs on Safari on macOS.
Moreover, every recent version of @material/web seems to be affected by this bug.

@asyncLiz could you take a look at this PR?

PS: Thanks for the awesome project!


Fixes: #5668

TextFields and Selects on Safari were showing a dark line, even when primary color was different.
Fixes: 5668
@asyncLiz
Copy link
Collaborator

asyncLiz commented Dec 3, 2024

Sorry for the long delay on this, and thank you for putting together a PR!

I did a bit of debugging on this, and I see the issue in an iOS 17.2 simulator. However, when I try inspecting and adding "border-color: inherit" to ".outline-notch", I see that the color is fixed. When I unfocus and re-focus the text field, it's broken again until I toggle "border-color: inherit" on and off.

Without any CSS changes on our catalog, I also see that the border color is incorrect after focusing, but "resets" and fixes itself when hovering.

Screen.Recording.2024-12-03.at.11.42.40.AM.mov

This is an indicator to me that there could be an underlying webkit style caching bug with pseudo elements, or an issue with how the border: inherit shorthand is applied when inheriting non-shorthand properties like border-color.

I'm hesitant to accept this PR until we know a bit more about why it's not working. We should confirm whether or not it's a webkit bug and file an issue if it is.

Do you mind investigating some with a standalone reproduction of this issue outside of our component? More than likely it's something to do with multiple levels of inheritance, pseudo elements, and/or border vs border-color.

Screenshot 2024-12-03 at 11 46 47 AM

@l0ll098
Copy link
Author

l0ll098 commented Feb 25, 2025

Hi @asyncLiz, sorry if this took ages but I was able to take a look into this only recently :(

To better understand what was going on within Safari, I've started from the definition of these components, and gradually removed things, ending up with the smallest reproducible component that presented the same issue.

By deleting a piece at a time, I was able to confirm that this is not caused by neither Lit, nor the Shadow DOM APIs, as I was able to reproduce the same behaviour by simply copying the component markup and including related styles in a "plain" HTML file. Also, by removing @layer declarations the same thing occurs, meaning that it's also not related to this CSS feature.

So, just to recap, by inspecting computed styles, both in Chrome and Safari, we can see some differences in how values are inferred, specifically by the element with the .outline-notch class.
In my tests I'm using some obscene colors to make things stand out a bit more, but essentially it boils down to using #79747e for the default outline color, #00ff00 for the outline color when field is hovered, and #ff00ff as outline color when the input field is focused.

So, when we focus the input field, we can see that .outline element correctly infers colors (#ff00ff), as also confirmed by these screenshots.

Screenshot 2025-02-25 alle 15 09 21

Screenshot 2025-02-25 alle 15 10 12

However, at this point we can clearly see that on Safari element with the outline-panel-active is still being rendered in green (#00ff00), rather than using the fucsia color (#ff00ff) I was expecting, as also seen in this screenshot. It's a short line because I don't have the label, but you get the idea ;)

Screenshot 2025-02-25 alle 15 21 39

At this point, since border colors of that .outline-panel-active element should be inferred by the parent element, I've inspected the .outline-notch element. Here we see the first difference in how values are inferred.
This is what Chrome does:

Screenshot 2025-02-25 alle 15 10 36

Whereas Safari is still using the #00ff00 color, as seen here:
Screenshot 2025-02-25 alle 15 10 26

Another interesting bit in this last screenshot is the value of the color property, which is correctly inferred. So the problem really seems to be the border property declared for the .outline-notch element.
Thinking it could be caused by an issue within Safari in expanding that shorthand property into its' corresponding properties, I've expanded those rules by hand into their corresponding longer rules, but the final result did not change, as the focused outline color was not being applied.

Also, the weird part is that both color and border-color properties for that .outline-notch element are updated by the same block of CSS rules, defined as follows. Yet, in Safari only changes to the color property seem to have an effect.

.focused .outline {
        border-color: var(--_focus-outline-color);
        color: var(--_focus-outline-color);
 }

It doesn't seem to be an issue with caching of CSS vars neither, as the same behaviour occurs also by swapping var(--_focus-outline-color); with an hard coded CSS named color, such as cadetblue.

The fact that the border-color seems to be ignored by Safari can be seen by inspecting the .outline-notch element and taking a look at computed styles. As seen also here, it appears as if the background-color had never been defined for that element.

Screenshot 2025-02-25 alle 15 57 51

Also, since we can see that the issue is already visible at this level in the DOM, the ::before and ::after pseudo elements do not have the chance of being rendered correctly.

Honestly at this point I was more confused than when I've started taking a look at the root cause.
So, I did some more tests. This time removing almost everything from the example and I was left with something like this:

Snippet
<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>

    <style>
        :root {
            /* Copied from "_outlined-styles", with some minor cleanup to keep things easy. */
            --_outline-color: #79747e;
            --_hover-outline-color: #00ff00;
            --_focus-outline-color: #ff00ff;
        }

        /* #region Some styles to make field larger. */
        #testWrapper {
            width: 20rem;
            height: 3rem;
            transform: scale(2, 2);
        }

        body {
            display: flex;
            flex-direction: column;
            align-items: center;
            align-content: center;
            justify-content: center;
            margin: 0;
            height: 100vh;
            width: 100vw;
        }

        input {
            width: 40rem;
            margin: 1rem;
            z-index: 1000;
        }

        /* #endregion */


        /* Copied from "_shared-styles.css" */
        .container-overflow {
            border-radius: 4px;
            display: flex;
            height: 100%;
            position: relative;
            resize: inherit;
        }

        .outline {
            border-color: var(--_outline-color);
            border-width: 1px;
            border-style: solid;
            border-radius: inherit;
        }

        #testWrapper:hover .outline {
            color: var(--_hover-outline-color);
            border-color: var(--_hover-outline-color);
        }

        #testWrapper.focused .outline {
            color: var(--_focus-outline-color);
            border-color: var(--_focus-outline-color);
            border-width: 2px;
        }


        .outline-start,
        .outline-end {
            border: inherit;
        }

        .outline-notch {
            border: inherit;
        }

        .outline-panel-inactive,
        .outline-panel-active {
            border: inherit;
        }

    </style>

    <script type="module">
        const wrapper = document.querySelector("#testWrapper");
        const input = document.querySelector("input");

        input.addEventListener("focus", () => {
            wrapper.classList.add("focused");
        });
        input.addEventListener("blur", () => {
            wrapper.classList.remove("focused");
        });
    </script>
</head>

<body>

    <div id="testWrapper">
        <div class="container-overflow">
            <div class="outline">
                <div class="outline-start">&nbsp;</div>
                <div class="outline-notch">
                    <div class="outline-panel-inactive">&nbsp;</div>
                    <div class="outline-panel-active">&nbsp;</div>
                </div>
                <div class="outline-end">&nbsp;</div>
            </div>

            <input type="text">
        </div>
    </div>

</body>

</html>

Here you can see something that resembles the content of the shadow root of a md-outlined-text-field text field. Basically, the #testWrapper element acts as the .field.populated div element in the shadow root of the MD text field. In the example we clearly do not have any <slot> and some other elements were removed, in the process of making everything simpler.
Also, we have to mimic what md-outlined-text-field does when focused, that is setting the focused class on the #testWrapper element when the input field gets focused, and removing that class when the blur event occurs. This is handled by the <script> at lines 88-98.

In the <style> element we set the same colors cited before for our text field, and we have some properties to make the input field larger and easier to see. Hence, nothing interesting here. The definition of the .container-overflow class is copied and pasted from the compiled field/internal/_shared.scss file. Again, nothing really interesting in here for this small demo. Then we have the .outline class. This is written by hand mimicking the original .outline class, but it has been stripped of every CSS rule and I left only border related properties, already expanded to their longer (yet not complete) versions. Essentially here we are setting some border color. With the CSS rules defined beneath that class, we have some more CSS selectors to change color and border-color when the input field gets focused or the root element is being hovered. Then we have only some selectors to inherit border properties.

The resulting element is an atrociously bad looking input field with an half broken border that presents the same characteristics of the original md-outlined-text-field element. In particular, when rendered in Chrome we get some overlapping borders that use the correct color (#ff00ff). When rendered by Safari, we get some borders that are still green (#00ff00) and some other borders that are fucsia (#ff00ff). Ideally, Safari should behave like Chrome and render the border using only one color.

Here's the result rendered by Chrome
Screenshot 2025-02-25 alle 16 57 15

and this is the result rendered by Safari:
Screenshot 2025-02-25 alle 16 57 29

Told you it was bad 😂


Seeing al of this, my theory is that Safari, for whatever reason, messes up with the inheritance/application of border-color properties when multiple CSS declaration blocks are involved in the update process of the rendered content. Indeed, if you comment the block #testWrapper:hover .outline, you will see that even Safari is able to color all borders using the correct fucsia color. So, I suspect that Safari is either caching or computing resulting styles of only one of the two blocks #testWrapper:hover .outline and #testWrapper.focused .outline.

@asyncLiz Does it make sense to you? Do you think is this worthy of a WebKit issue?

Also, I was wondering if we could do something in this project to work around this issue: even if this was confirmed to be a WebKit bug, it would probably take some time before being fixed. Also, that fix would likely apply only to newer versions of Safari, resulting in current and older Safari version seeing the broken border colors, and ultimately ruining the UX of those pages using these components. Is there something in this PR you would like to be refactored/rewritten in another way before eventually proceeding with the merge?

Edit: here you can find the HTML page I was using for the first set of tests, if you want to play with it

Snippet for first tests
<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>

    <style>
        :root {
            /* Copied from "_outlined-styles", with some minor cleanup to keep things easy. */
            --_outline-color: #79747e;
            --_hover-outline-color: #00ff00;
            --_focus-outline-color: #ff00ff;

            /* 
                These are not strictly required, however, if not defined, we won't be able to see the 
                issue in the rendered page and we have to look at inferred colors using devtools. 
            */
            --_TEST_container-shape: 4px;
            --_leading-space: var(--md-outlined-field-leading-space, 16px);
            --_outline-label-padding: var(--md-outlined-field-outline-label-padding, 4px);
        }

        /* #region Some styles to make field larger. */
        #testWrapper {
            width: 20rem;
            height: 3rem;
            transform: scale(2, 2);
        }

        body {
            display: flex;
            flex-direction: column;
            align-items: center;
            align-content: center;
            justify-content: center;
            margin: 0;
            height: 100vh;
            width: 100vw;
        }

        input {
            width: 40rem;
            margin: 1rem;
            z-index: 1000;
        }
        /* #endregion */

        /* Copied from "_shared-styles.css" */
        .container-overflow {
            border-radius: 4px;
            display: flex;
            height: 100%;
            position: relative;
            resize: inherit;
        }

    
        /* ------ */
        .outline {
            border-color: var(--_outline-color);
            border-radius: inherit;
            display: flex;
            pointer-events: none;
            height: 100%;
            position: absolute;
            width: 100%;
            z-index: 1;
        }

        .outline-start::before,
        .outline-start::after,
        .outline-panel-inactive::before,
        .outline-panel-inactive::after,
        .outline-panel-active::before,
        .outline-panel-active::after,
        .outline-end::before,
        .outline-end::after {
            border: inherit;
            content: "";
            inset: 0;
            position: absolute;
        }

        .outline-start,
        .outline-end {
            border: inherit;
            border-radius: inherit;
            box-sizing: border-box;
            position: relative;
        }

        .outline-start::before,
        .outline-start::after,
        .outline-end::before,
        .outline-end::after {
            border-bottom-style: solid;
            border-top-style: solid;
        }

        .outline-start::after,
        .outline-end::after {
            opacity: 0;
        }

        #testWrapper.focused .outline-start::after,
        #testWrapper.focused .outline-end::after {
            opacity: 1;
        }

        .outline-start::before,
        .outline-start::after {
            border-inline-start-style: solid;
            border-inline-end-style: none;
            border-start-start-radius: inherit;
            border-start-end-radius: 0;
            border-end-start-radius: inherit;
            border-end-end-radius: 0;
            margin-inline-end: var(--_outline-label-padding);
        }

        .outline-end {
            flex-grow: 1;
            margin-inline-start: calc(-1*var(--_outline-label-padding));
        }

        .outline-end::before,
        .outline-end::after {
            border-inline-start-style: none;
            border-inline-end-style: solid;
            border-start-start-radius: 0;
            border-start-end-radius: inherit;
            border-end-start-radius: 0;
            border-end-end-radius: inherit;
        }

        .outline-notch {
            align-items: flex-start;
            border: inherit;
            display: flex;
            margin-inline-start: calc(-1*var(--_outline-label-padding));
            margin-inline-end: var(--_outline-label-padding);
            max-width: calc(100% - var(--_leading-space) - var(--_trailing-space));
            padding: 0 var(--_outline-label-padding);
            position: relative;
        }

        .outline-panel-inactive,
        .outline-panel-active {
            border: inherit;
            border-bottom-style: solid;
            inset: 0;
            position: absolute;
        }

        .outline-panel-inactive::before,
        .outline-panel-inactive::after,
        .outline-panel-active::before,
        .outline-panel-active::after {
            border-top-style: solid;
            border-bottom: none;
            bottom: auto;
        }

        .outline-start {
            padding-inline-start: max(var(--_leading-space), var(--_TEST_container-shape) + var(--_outline-label-padding));
        }

        .outline-start::before,
        .outline-end::before,
        .outline-panel-inactive,
        .outline-panel-inactive::before,
        .outline-panel-inactive::after {
            border-width: var(--_outline-width);
        }

        #testWrapper:hover .outline {
            border-color: var(--_hover-outline-color);
            color: var(--_hover-outline-color);
        }

        #testWrapper:hover .outline-start::before,
        #testWrapper:hover .outline-end::before,
        #testWrapper:hover .outline-panel-inactive,
        #testWrapper:hover .outline-panel-inactive::before,
        #testWrapper:hover .outline-panel-inactive::after {
            border-width: var(--_hover-outline-width);
        }

        #testWrapper.focused .outline {
            border-color: var(--_focus-outline-color);
            color: var(--_focus-outline-color);
        }

        .outline-start::after,
        .outline-end::after,
        .outline-panel-active,
        .outline-panel-active::before,
        .outline-panel-active::after {
            border-width: var(--_focus-outline-width);
        }
    </style>

    <script type="module">
        const wrapper = document.querySelector("#testWrapper");
        const input = document.querySelector("input");

        input.addEventListener("focus", () => {
            wrapper.classList.add("focused");
        });
        input.addEventListener("blur", () => {
            wrapper.classList.remove("focused");
        });
    </script>
</head>

<body>
    <div id="testWrapper">
        <div class="container-overflow">
            <div class="outline">
                <div class="outline-start"></div>
                <div class="outline-notch">
                    <div class="outline-panel-inactive"></div>
                    <div class="outline-panel-active"></div>
                </div>
                <div class="outline-end"></div>
            </div>

            <input type="text">
        </div>
    </div>
</body>

</html>

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.

Text Field Outline/Border Color breaks under Label into a Black Segment
2 participants