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.

Sign up for GitHub

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

Jump to bottom

Use Voila logo for the spinner #393

Merged
merged 3 commits into from Oct 8, 2019
Merged

Use Voila logo for the spinner #393

merged 3 commits into from Oct 8, 2019

Conversation

martinRenou
Copy link
Member

spinner

@jtpio
Copy link
Member

jtpio commented Oct 2, 2019

That looks nice.

Maybe we want to minify / compress the svg file too ?

@martinRenou
Copy link
Member Author

We can try, but it looks pretty small already

@SylvainCorlay
Copy link
Member

This is really cool!

The improvement of the spinner SVG should probably be in a different PR. (Removing the green circle under the yellow will remove the rendering artifact)

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 2, 2019 •
edited

A way to prevent the loading delay may be to inline the SVG in the XML.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 2, 2019 •
edited

One simple change to make that will reduce the size would be

  • to remove all the attributes prefixed with inkscape in the SVG, as well as the definition of the inkscape xml namespace.
  • same for sodipodi
  • same for id="Calque_1", data-name="Calque 1".

@jtpio
Copy link
Member

jtpio commented Oct 2, 2019

Inkscape has an option to save as "Optimized SVG (*.svg)", which should take care of all these.

@martinRenou
Copy link
Member Author

I also removed unused defined colors, whitespaces and new lines, saving ~100kb

@jtpio
Copy link
Member

jtpio commented Oct 3, 2019

Does this affect the perceived loading delay?

@martinRenou
Copy link
Member Author

Does this affect the perceived loading delay?

It does actually, the logo arrived the same time as the text when I tried. But it's only local...

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 3, 2019 •
edited

Does this affect the perceived loading delay?

The way to fix this is to inline the svg in the HTML.

This will also make it possible to set the colors of the parts of the spinner with CSS.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 3, 2019 •
edited

This looks great and I am happy to get this in as it is now.
Leaving it open to give a chance to @maartenbreddels to chime in.

@martinRenou
Copy link
Member Author

Some CSS fixes coming

@jtpio
Copy link
Member

jtpio commented Oct 8, 2019

Anything more to add / fix before merging it? (so it can go in the next release)

@SylvainCorlay
Copy link
Member

We should credit Jasmine for her contribution!

@maartenbreddels
Copy link
Member

Perfect, especially inlining the svg indeed, that should render instantly.

@martinRenou
Copy link
Member Author

@jasminefrh I added your contribution :) Thanks a lot for your help!

@SylvainCorlay SylvainCorlay merged commit 857d0b7 into voila-dashboards:master Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SylvainCorlay SylvainCorlay SylvainCorlay left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@martinRenou @jtpio @SylvainCorlay @maartenbreddels @gigiland