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

Visual helper for text spacing fails to change the spacing for text in input fields and buttons #4800

Open
Sciurus7 opened this issue Oct 18, 2021 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team.

Comments

@Sciurus7
Copy link

Visual helper for text spacing fails to change the spacing for text in input fields and buttons

To Reproduce
Steps to reproduce the behavior:

  1. Open a page with at least a native input field and a button, enter some text in the input field.
  2. Start an Accessibility Insights for Web assessment.
  3. Navigate to step 17.6, Adaptable content > Text spacing.
  4. Toggle the Visual helper toggle to the "On"state.
  5. The spacing of the text in the input field and on the button does not change.

Expected behavior

The spacing of the text in the input field and on the button does change.

Screenshots

Before:
image

After:
image

Context (please complete the following information)

  • Browser Version: Google Chrome Version 94.0.4606.81 (Official Build) (32-bit)
  • Target Page: https://www.wikipedia.com will suffice as target for the assessment

Are you willing to submit a PR?

Did you search for similar existing issues?

Yes

Additional context

This bug is caused because the tool attempts to use inheritance to change text spacing. This however is very weak and easily overridden. A colleague of mine created an html code snippet which pins down the underlying problem:

<style>
    *:not(.not-universal, input) {
        color: blue;
    }
    .container {
        color: fuchsia !important;
    }
    .test {
        color: green;
    }
</style>
<div class="container">
    This is the exact element, i.e. no inheritance, so fuchsia
    <p>
        This is blue because the universal selector is stronger than inheritance
    </p>
    <p class="test">
        This is green because the class selector is stronger than inheritance.
        <span class="not-universal">
            Still green, because the "test" class is closer. (And the universal selector is explicitly not applied here)
        </span>
    </p>
    <input value="black due to user agent">
</div>
@Sciurus7 Sciurus7 added the bug Something isn't working label Oct 18, 2021
@ferBonnin ferBonnin added the status: new This issue is new and requires triage by DRI. label Oct 18, 2021
@jalkire jalkire added the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Oct 18, 2021
@ghost ghost assigned ferBonnin Oct 18, 2021
@ghost
Copy link

ghost commented Oct 18, 2021

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@ghost ghost removed the status: new This issue is new and requires triage by DRI. label Oct 18, 2021
@jalkire jalkire removed their assignment Oct 18, 2021
@jalkire
Copy link
Contributor

jalkire commented Oct 18, 2021

I was able to reproduce this.

@ferBonnin
Copy link
Contributor

triaged with Peter, needs investigation to understand how to fix this issue

@ferBonnin ferBonnin added the status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team. label Oct 25, 2021
@ghost
Copy link

ghost commented Oct 25, 2021

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

@ghost ghost removed the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label Oct 25, 2021
@ThanyaLeif ThanyaLeif self-assigned this Oct 12, 2022
@ThanyaLeif ThanyaLeif added the status: active This issue is currently being worked on by someone. label Oct 12, 2022
@ghost ghost removed the status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team. label Oct 12, 2022
@ThanyaLeif ThanyaLeif added status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team. and removed status: active This issue is currently being worked on by someone. labels Oct 17, 2022
@ThanyaLeif ThanyaLeif removed their assignment Oct 17, 2022
@ghost
Copy link

ghost commented Oct 17, 2022

This issue requires additional investigation by the Accessibility Insights team. When the issue is ready to be triaged again, we will update the issue with the investigation result and add "status: ready for triage". Thank you for contributing to Accessibility Insights!

@peterlukerow
Copy link

You could you the same method as the JS bookmarklet that was reference in #6553 which does work that was closed as a duplicate of this.

@peteretelej
Copy link
Member

The injected styles are overridden by browser default styling (for example for input)
image

overridden by
image

More specific selector for example will correctly show change in spacing as it won't be overridden (wildcard here just for an example)
image

@JGibson2019 JGibson2019 added help wanted Extra attention is needed good first issue Good for newcomers labels Feb 5, 2024
@seanelliott86
Copy link

I've also seen this have trouble changing headings when testing against libraries using css-in-js or elements that have classes attached that define the line-height, letter-spacing, and word-spacing properties.

It would be possible to change everything by leveraging the current injected class and targeting all elements:

.insights-formatted-text-spacing-container *{
    line-height: 1.5 !important;
    letter-spacing: 0.12em !important;
    word-spacing: 0.16em !important;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed status: needs investigation This issue requires investigation (PM, Design, or SWE) by the Accessibility Insights team.
Projects
Status: Accepted - external
Development

No branches or pull requests

8 participants