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

WCAG 2.1 1.4.12 Text spacing in the Switcher #1219

Closed
ferBonnin opened this issue Sep 6, 2019 · 16 comments

Comments

@ferBonnin
Copy link
Contributor

@ferBonnin ferBonnin commented Sep 6, 2019

Describe the bug

The switcher to change from fastpass to assessment has overlapping text

To Reproduce

  1. Go to text legibility> text spacing
  2. run the following function (Same as the bookmarklet)
    (function () { document.querySelector("body").classList.add("text-spacing"); var style = document.createElement("style"), styleContent = document.createTextNode(".text-spacing * { line-height: 1.5 !important; letter-spacing: 0.12em !important; word-spacing: 0.16em !important; } .text-spacing p{ margin-bottom: 2em !important; } "); style.appendChild(styleContent); var caput = document.getElementsByTagName("head"); caput[0].appendChild(style); })();
  3. check the switcher

Expected behavior

all text is visible

Screenshots

Switcher with overlapping text

Extension (please complete the following information)

  • OS: [e.g. Windows/Mac] Windows
  • Version [e.g. 1.140.1] 2.9.1
  • Browser Version [e.g Chrome 71.0] 75
  • Target Page [e.g www.bing.com] any

Are you willing to submit a PR?

No

Did you search for similar existing issues?

Yes

Additional context

@msft-github-bot

This comment has been minimized.

Copy link
Collaborator

@msft-github-bot msft-github-bot commented Sep 9, 2019

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!

@msft-github-bot

This comment has been minimized.

Copy link
Collaborator

@msft-github-bot msft-github-bot commented Sep 9, 2019

The team requires additional author feedback; please review their replies and update this issue accordingly. Thank you for contributing to Accessibility Insights!

@AhmedAbdoOrtiga

This comment has been minimized.

Copy link
Contributor

@AhmedAbdoOrtiga AhmedAbdoOrtiga commented Sep 9, 2019

I can't seem to repro, the only difference is my Chrome version is "Version 76.0.3809.132 (Official Build) (64-bit)", can you please advise?

@msft-github-bot

This comment has been minimized.

Copy link
Collaborator

@msft-github-bot msft-github-bot commented Sep 10, 2019

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!

@AhmedAbdoOrtiga

This comment has been minimized.

Copy link
Contributor

@AhmedAbdoOrtiga AhmedAbdoOrtiga commented Sep 10, 2019

Worked with @peterdur, we were able to repro it by running the code in the DevTool console

@ferBonnin ferBonnin added this to Ready for triage in Accessibility Insights Sep 18, 2019
@ferBonnin ferBonnin removed their assignment Sep 23, 2019
@ferBonnin ferBonnin moved this from Ready for triage to Ready for work bugs in Accessibility Insights Sep 23, 2019
@peterdur

This comment has been minimized.

Copy link
Member

@peterdur peterdur commented Oct 16, 2019

Looks to be related to .ms-Dropdown-title having width: 139px;

@varshannagarajan

This comment has been minimized.

Copy link
Contributor

@varshannagarajan varshannagarajan commented Oct 24, 2019

Hey, I was wondering if I could work on this issue.

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Oct 24, 2019

Hi @varshannagarajan for sure! thanks for the help :)

varshannagarajan added a commit to varshannagarajan/accessibility-insights-web that referenced this issue Oct 30, 2019
@varshannagarajan

This comment has been minimized.

Copy link
Contributor

@varshannagarajan varshannagarajan commented Oct 30, 2019

Hi, @ferBonnin I was trying out a few different changes and was wondering if the drop down box should remain the same size or if it was OK if it changed according to the length of the text?

Static:
StaticDropdown

Dynamic:
DynamicDropdown

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Oct 30, 2019

hi @varshannagarajan thanks for adding the gif and experimenting with different ideas :)
I think it would be better to have the dropdown box size static. That part of the UI doesn't really change much for the users and it feels more consistent if the size doesn't change.

I noticed from the gif that the dropdown is a bit bigger than the title bar, is it just the gif? if not, could you please make sure the dropdown stays contained in the title bar?
screenshot of the ddl, it looks slightly overflow

thanks again for the awesome contribution!

@ferBonnin ferBonnin moved this from Ready for work bugs to Active in Accessibility Insights Oct 30, 2019
@varshannagarajan varshannagarajan mentioned this issue Nov 1, 2019
3 of 7 tasks complete
@varshannagarajan

This comment has been minimized.

Copy link
Contributor

@varshannagarajan varshannagarajan commented Nov 1, 2019

Hi , I have submitted a pull request to just address the text overlap. I tried a couple of different things to make sure the drop down box would not go below the header (when the function is applied) but wasn't yet able to find a good solution. I was wondering if it would be OK to change the height of the header or if that must stay the same?

I submitted the PR now just to get it in before the end of Hacktoberfest, I hope that is alright. I will continue to work on the shifting issue and I will submit a PR for it.

Also I did notice when the function is applied on the side menu the text is cut off.

image

I believe is would also be a problem with WCAG 2.1 1.4.12 and was wondering if you would like me to address that as well.

And no problem I am happy to help!

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Nov 1, 2019

Hi @varshannagarajan thanks for the PR! Since you plan on keep working on this, I have marked your PR as do not merge for now until we are ready, sounds good?

I am adding @peterdur to the thread, can we help with varshannagarajan to fix the drop down box without changing the size of the header?

for the side menu, great point! if you want to file an issue and help us fix it that would be great :)

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Nov 11, 2019

Marking this as resolved :)

@ferBonnin ferBonnin self-assigned this Nov 11, 2019
@ferBonnin ferBonnin moved this from Active to Blocked in Accessibility Insights Nov 11, 2019
@varshannagarajan

This comment has been minimized.

Copy link
Contributor

@varshannagarajan varshannagarajan commented Nov 14, 2019

Hi, I was wondering if you would like me to create a new separate issue regarding the drop down box moving outside the header?

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Dec 4, 2019

validated this issue in canary and insider, looks good :)

@ferBonnin

This comment has been minimized.

Copy link
Contributor Author

@ferBonnin ferBonnin commented Dec 4, 2019

Hi, I was wondering if you would like me to create a new separate issue regarding the drop down box moving outside the header?

yes, please!

@ferBonnin ferBonnin closed this Dec 4, 2019
Accessibility Insights automation moved this from Blocked to Closed Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5 participants
You can’t perform that action at this time.