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 loading icon bug on IE #5460

Merged
merged 1 commit into from
May 14, 2016
Merged

Conversation

scott1702
Copy link
Contributor

Unfortunately IE can't handle css animating SVGs, this just turns the into s so we can animate them in IE.

Fixes #5283

@chillu chillu self-assigned this May 6, 2016
@scott1702 scott1702 force-pushed the btn-loading-icon branch 3 times, most recently from 42e9818 to d33e3c4 Compare May 9, 2016 05:55
@chillu
Copy link
Member

chillu commented May 9, 2016

@scott1702 This hasn't been tested with other button style variations I think, the dots are grey regardless of the button text colour (cc @clarkepaul)

image

@scott1702 scott1702 force-pushed the btn-loading-icon branch 3 times, most recently from ba6925a to 67e2e37 Compare May 10, 2016 21:14
@scott1702
Copy link
Contributor Author

Should be good now @chillu

@chillu
Copy link
Member

chillu commented May 10, 2016

Broken up in two lines for IE10 on "Edit Campaign": http://take.ms/8rzqa

@scott1702
Copy link
Contributor Author

Talked this over with @clarkepaul around how we could handle this, came to the decision of using the same indicator with smaller circles. This change has been added and tested with buttons as small as 'icon-only'.

Third time lucky @chillu? 😉

@clarkepaul
Copy link
Contributor

Looked good to me and tested in IE10.

}

.btn-danger {
.btn__circle {
Copy link
Contributor

@dhensby dhensby May 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're not meant to nest elements in BEM

ergh, ignore me this is inside a bootstrap type modified

@dhensby
Copy link
Contributor

dhensby commented May 12, 2016

needs rebasing :/

@scott1702
Copy link
Contributor Author

Rebased :)

@dhensby dhensby merged commit ca4341f into silverstripe:master May 14, 2016
@dhensby
Copy link
Contributor

dhensby commented May 14, 2016

Thanks

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.

None yet

4 participants