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

CSS: clean up some classes #74

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

CSS: clean up some classes #74

wants to merge 3 commits into from

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Sep 1, 2020

There were some values hard-coded in the html attributes,
but we already were setting them in our css.

Also, search__backdrop was basically the same as search__outer__wrapper

You can see it working at https://readthedocs-sphinx-search.readthedocs.io/en/clean-up-css/?rtd_search=search

@agjohnson
Copy link

It might be worth testing IE/Edge for this, as the attributes on the svg element might be required for IE compatibility. IE support for svg was bad for a long time, and scaling issues are a known problem.

There were some values hard-coded in the html attributes,
but we already were setting them in our css.

Also, search__backdrop was basically the same as search__outer__wrapper
@stsewd
Copy link
Member Author

stsewd commented Dec 17, 2020

According to this thing it looks good https://www.browserling.com/browse/win/7/ie/11/https%3A%2F%2Freadthedocs-sphinx-search--74.org.readthedocs.build%2Fen%2F74%2F I may need to boot a windows VM to double check, don't think there is a headless driver for IE

@agjohnson
Copy link

We also have a OSS account at saucelabs that i commonly use for theme development, I can share the creds with you

@stsewd
Copy link
Member Author

stsewd commented Jan 4, 2021

@agjohnson yes please!

@stsewd
Copy link
Member Author

stsewd commented Jan 6, 2021

@agjohnson looks like everything works as expected. Noticed the extension doesn't work at all on IE 10, not sure if we want to support it though.

@agjohnson
Copy link

Can you post a screen grab or something for ie10? i remember there were some issues, but it should work.

We've talked about dropping support for ie10, but don't have an official position yet. Last I checked, there was still a non-zero amount of ie10 traffic -- which when you're talking about 20M monthly pageviews, is still probably ~100k monthly pageviews. I've lost where we had the conversation last

@stsewd
Copy link
Member Author

stsewd commented Jan 6, 2021

Can you post a screen grab or something for ie10? i remember there were some issues, but it should work.

This UI is the same, search doesn't work, it throws an exception in the js code, so when you search nothing happens, and you can't even close the modal

Screenshot_2021-01-06 Sauce Labs

the docs.rtfd.io site has definitely problems with the svg image :D

Screenshot_2021-01-06 Sauce Labs(1)

@stsewd stsewd requested a review from agjohnson January 19, 2021 23:47
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.

2 participants