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

Switch to simpler looking loading spinner #24107

Closed
wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Apr 20, 2016

New spinner.
See discussion in ref issue.

Not compatible with ie (sigh) so i've added a fallback to gif!
I also added a small black loading icon: icon-loading-small-dark

ref #19091
@oparoz @Henni @jancborchardt @PVince81

@skjnldsv skjnldsv self-assigned this Apr 20, 2016
@skjnldsv skjnldsv added this to the 9.1-current milestone Apr 20, 2016
@ChristophWurst
Copy link
Contributor

On Firefox, the spinner does not spin.

@skjnldsv
Copy link
Contributor Author

Dammit! It works with direct image loading, but not as background-image... :/

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 20, 2016

Thanks @ChristophWurst ! :)
Okay, I switched from css animation to smil, it seems to be accepted as well as css animations:
http://caniuse.com/svg-smil

We still have the gif fallback for ie!

@oparoz
Copy link
Contributor

oparoz commented Apr 20, 2016

Can't use SMIL

As of Chrome 45 SMIL is deprecated and usage will result in a warning in the console. Support is expected to be dropped in some future version.

@skjnldsv
Copy link
Contributor Author

I'm gonna stop coding now, and raise goats somewhere in the alps...

@oparoz
Copy link
Contributor

oparoz commented Apr 20, 2016

I'm gonna stop coding now, and raise goats somewhere in the alps...

Not everybody likes goat milk, you're going to need to include a hamster milk fallback

@mmattel
Copy link
Contributor

mmattel commented Apr 20, 2016

I'm gonna stop coding now, and raise goats somewhere in the alps...

This is a problem. There is no more free space left in the alps to raise goats. Too many frustrated programmers occupied everything. But there are maybe free caves availabe for meditation 😄

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 20, 2016

Okay, after a day of thinking while milking my goats, we should go for smil.
Considering the support still present for the next upcoming chrome & ff versions, we should be good for a while. Maybe the css-svg bug in ff will be fixed when smil will be dropped! :)
Chrome most used is 48 then 49, and ff is 44 then 45.

can i use support tables for html5 css3 etc

Let's review @oparoz @Henni @jancborchardt @PVince81 @MorrisJobke @ChristophWurst !
Procedure:

  • Install commit
  • Just use owncloud and search for an inconvenient loader. Mails, contacts, user menu, files, gallery...

@skjnldsv
Copy link
Contributor Author

For future reference:
This is also another way of using after:

.icon-loading::after {
    position: absolute;
    content: url('../img/loading-css.svg');
    width: 100%;
    height: 100%;
    top:50%;
    left: 50%;
    margin: -25px 0 0 -25px;
}

@MorrisJobke
Copy link
Contributor

Just use owncloud and search for an inconvenient loader. Mails, contacts, user menu, files, gallery...

Add sleep(5); to apps/dav/appinfo/v1/webdav.php and load the web UI ;)

@MorrisJobke
Copy link
Contributor

Add sleep(5); to apps/dav/appinfo/v1/webdav.php and load the web UI ;)

Also the while waiting for the search results to come in seems to be an old one. Beside that it looks awesome :)

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 21, 2016

@MorrisJobke here you go! :D

@MorrisJobke
Copy link
Contributor

Tested and works so far. I would merge this in and then fix the remaining stuff afterwards.

👍

vertical-align: middle;
margin-left: 10px;
float: left;
padding: 3px 10px;
Copy link
Member

Choose a reason for hiding this comment

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

Here too, last line indent is wrong.

@jancborchardt
Copy link
Member

Very nice! :) Except the indentation stuff and the spinner in Files, everything looks much better now!

@skjnldsv
Copy link
Contributor Author

@jancborchardt Better now? Is it spinning on ff?

@ChristophWurst
Copy link
Contributor

Works, but only randomly on FF. When the image is not cached, it does not spin for some reason, which looks odd. :-/

@skjnldsv
Copy link
Contributor Author

Should we assume the user will have loaded the svg at first and that it will be spinning on the rest of his browsing?

@skjnldsv
Copy link
Contributor Author

@jancborchardt Files fixed.

@jancborchardt
Copy link
Member

Nice! 👍 now only squash and this is good to merge

@oparoz
Copy link
Contributor

oparoz commented Apr 22, 2016

Doesn't work Almost works on Firefox

@oparoz
Copy link
Contributor

oparoz commented Apr 22, 2016

Actually, that's not true. The spinner just freezes sometimes, but that's normal FF behaviour. With an empty cache, the first spinner is always empty.

I have one concern though, the spinner itself doesn't really work that well in a lot of situations, especially when the 16px version is used. It's a bit too discreet. It's too thin.

@jancborchardt
Copy link
Member

So for the 16px version we might need a dedicated one – the line thickness should be the same as for the big spinner (3px?), not scaled down.

The Firefox issues I would say we should fix going forward, otherwise this will sit for too long. :\

@oparoz
Copy link
Contributor

oparoz commented Apr 22, 2016

So for the 16px version we might need a dedicated one – the line thickness should be the same as for the big spinner (3px?), not scaled down.

Sounds sane. I'm still worried about the big one as well, it's really thin compared to what's out there. There may be cases, especially when use as an overlay where it might not work that well.

The Firefox issues I would say we should fix going forward, otherwise this will sit for too long. :\

I think we should try pre-loading as you suggested. I was able to reproduce the problem after clearing the cache. It's quite obvious when you switch app after logging in.

@MorrisJobke
Copy link
Contributor

Looks really nice and works now in Firefox too :)

@MorrisJobke
Copy link
Contributor

👍

@@ -2507,6 +2485,7 @@
}
});
fileUploadStart.on('fileuploadadd', function(e, data) {
console.log('XXXXXXX');
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, don't know how this got there! :O

@skjnldsv
Copy link
Contributor Author

So, should we preload for ff?

@jancborchardt
Copy link
Member

If that fixes the last issue, go for it! :)

@skjnldsv
Copy link
Contributor Author

I will try, and we'll see :)

@skjnldsv skjnldsv force-pushed the new-animated-svg-spinner branch 3 times, most recently from df47dde to 3796b58 Compare April 26, 2016 05:04
- Fix search spinner
- Fix reset password loader
- Fix login spinner
- Fix ldap settings spinner
- Fix files loader
- Fix app loading header
@skjnldsv
Copy link
Contributor Author

skjnldsv commented May 9, 2016

Closed in favor of #24491

@skjnldsv skjnldsv closed this May 9, 2016
@MorrisJobke MorrisJobke deleted the new-animated-svg-spinner branch May 9, 2016 06:15
@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.

7 participants