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

New animated loader: full css #24491

Merged
merged 1 commit into from
May 9, 2016
Merged

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented May 9, 2016

New spinner. This time we're going full css. This is the best solution.
See discussion in ref issue.

We have a gif fall-back for css replaced elements (inputs, img...). This should easily do the trick, and is fully compatible with all browsers (including IE \o/)
I also added a small black loading icon: icon-loading-small-dark

ref and fix #19091
@oparoz @Henni @jancborchardt @PVince81

@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

Seems to have an issue with some of the gifs. I'm checking it.

@skjnldsv skjnldsv force-pushed the new-animated-full-css-spinner branch from 4b4a45f to ea3cc26 Compare May 9, 2016 05:55
@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

Ok, fixed now! :)
Gifs are now optimised and a bit more visible. Obviously, the render isn't as good as the full css, but this is a fallback after all ;)

@MorrisJobke
Copy link
Contributor

Ready for reviews?

@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

@MorrisJobke
Yes! :)

@MorrisJobke MorrisJobke added this to the 9.1-current milestone May 9, 2016
@ChristophWurst
Copy link
Contributor

When opening a text file with the text editor, the loading spinner stays forever. Does not happen with the old spinner on master.
Apart from that it looks very nice! Tested with FF on Linux

skjnldsv added a commit to owncloud/files_texteditor that referenced this pull request May 9, 2016
@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

See owncloud/files_texteditor#173 for the text loading bug.
Not related to this commit:

<skjnldsv> ok I think I see why
<skjnldsv> the class stays
<skjnldsv> but the textarea just go over it
<skjnldsv> so the loading is not visible
<skjnldsv> but now, because we're not using a background but a separated css element (::after) its stays on top
<ChristophWurst> ah, so it's just a coincidence it worked with the old one? :D
<skjnldsv> exactly ^^

@ChristophWurst
Copy link
Contributor

👍 then, no other problems detected

@MorrisJobke
Copy link
Contributor

Tested, looks really nice and works also in IE11 👍 😃

@MorrisJobke
Copy link
Contributor

@DeepDiver1975 JSUnit tests pass and the other stuff is CSS only. Could you merge this?

@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

Finally! \o/

@DeepDiver1975 DeepDiver1975 merged commit 885c842 into master May 9, 2016
@DeepDiver1975 DeepDiver1975 deleted the new-animated-full-css-spinner branch May 9, 2016 09:32
@jancborchardt
Copy link
Member

Nice dude, great work! :)

@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

Thanks @jancborchardt !
Took a while to figure out the best.

@oparoz
Copy link
Contributor

oparoz commented May 9, 2016

Well done @skjnldsv - This was merged rather quickly, so let's hope enough people will be able to test it before launch to offer feedback. I still think the ring is a bit thin which makes it too discreet in some situations. We'll see :)

@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

We already review the colors previously with jan!
I think this is alright (except the gif), we'll see if something comes up :)

@jancborchardt
Copy link
Member

Yeah, now we have enough time to see it in action so it's good we finally merged it! :)

skjnldsv pushed a commit to owncloud/calendar that referenced this pull request May 10, 2016
skjnldsv pushed a commit to owncloud/calendar that referenced this pull request May 10, 2016
skjnldsv pushed a commit to owncloud/calendar that referenced this pull request May 10, 2016
georgehrke added a commit to owncloud/calendar that referenced this pull request May 10, 2016
@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to simpler looking loading spinner
6 participants