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

Change loading animation frame order #10358

Merged
merged 3 commits into from
Feb 19, 2020
Merged

Change loading animation frame order #10358

merged 3 commits into from
Feb 19, 2020

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Feb 19, 2020

image

@lafriks lafriks added topic/ui Change the appearance of the Gitea UI skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Feb 19, 2020
@lafriks lafriks added this to the 1.12.0 milestone Feb 19, 2020
@lafriks lafriks added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Feb 19, 2020
@lunny
Copy link
Member

lunny commented Feb 19, 2020 •
edited
Loading

The original animated image hints that you could go to drink a cup of tea now. :) When you are drining tea, it will be less and less.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 19, 2020
silverwind
silverwind approved these changes Feb 19, 2020
View reviewed changes
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Filling cup seems more appropriate to me given that this represents a loading bar.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 19, 2020
@lafriks
Copy link
Member Author

lafriks commented Feb 19, 2020

@lunny I did not thought about such perspective :) For me it just looked awkward that data is being imported but tea is getting less and less :)

@codecov-io
Copy link

Codecov Report

Merging #10358 into master will increase coverage by 0.01%.
The diff coverage is 55.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10358      +/-   ##
==========================================
+ Coverage   43.69%    43.7%   +0.01%     
==========================================
  Files         585      585              
  Lines       81023    81023              
==========================================
+ Hits        35399    35414      +15     
+ Misses      41236    41224      -12     
+ Partials     4388     4385       -3
Impacted Files Coverage Δ
models/models.go 61.93% <ø> (ø) ⬆️
modules/markup/markup.go 76.81% <ø> (ø) ⬆️
modules/markup/markdown/goldmark.go 66.07% <ø> (ø) ⬆️
modules/indexer/issues/bleve.go 64.88% <ø> (ø) ⬆️
models/migrate.go 0% <0%> (ø) ⬆️
services/pull/commit_status.go 5% <0%> (ø) ⬆️
models/attachment.go 57.3% <100%> (ø) ⬆️
models/user.go 49.54% <100%> (ø) ⬆️
modules/queue/workerpool.go 56.93% <100%> (ø) ⬆️
modules/setting/indexer.go 91.66% <100%> (ø) ⬆️
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21e2d6b...0ed40a0. Read the comment docs.

6543
6543 approved these changes Feb 19, 2020
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

I asked a "normal" person - he choose the new one :) so lgtm

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 19, 2020
@silverwind
Copy link
Member

New one is also 30% smaller, I wonder why 😉

@lafriks
Copy link
Member Author

lafriks commented Feb 19, 2020

Could be because of different tool used, I don't know :) I used Linux cli tool apngasm

@silverwind
Copy link
Member

Too bad we can't optimize it further, both pngcrush and zopflipng destroy the animation.

@silverwind
Copy link
Member

silverwind commented Feb 19, 2020 •
edited
Loading

Actually managed to squeeze out 500 bytes using apngopt:

This image may be a good candidate for SVG SMIL animation. I guess smooth animation could be possible using spline animation 😉

@lafriks lafriks merged commit e461852 into go-gitea:master Feb 19, 2020
@lafriks lafriks deleted the feat/loading branch February 19, 2020 21:00
@silverwind
Copy link
Member

Not sure why you landed that but I opened #10372 for the optimization.

@lafriks
Copy link
Member Author

lafriks commented Feb 19, 2020

I did not thought that you wanted me to change it :)

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

@silverwind silverwind silverwind approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/ui Change the appearance of the Gitea UI type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
6 participants
@lafriks @lunny @codecov-io @silverwind @6543 @GiteaBot