Stop logo clashing when pinwheel animates #35564

caitpj · 2023-11-10T00:06:43Z

When hovering over the Airflow logo in the top left, the pinwheel part of the logo cuts across the text part of the logo. This is (subjectively) poor design. In order to stop this from happening, while keeping the pinwheel animation, this PR has:

set text to transition slightly to the right when the animation activates.

I have also set the pinwheel animation back to infinite. In my opinion, having read the discussion that resulted in the change, it was not the correct decision to switch the animation from infinite over hover to 1.5 seconds. Another change was made, to allow users with a 'prefers-reduced-motion' preference to not have the animation playing, was the correct change, and did not need a further change to the length of time the pinwheel animates for.

This is a design change, which is always subjective, so totally understand if review wants to bin this. Also sorry in advance for anything bad I've done; it's my 1st PR, I promise to do better next time.

Video demonstrating change:
Before

282189311-b7d3e49c-4f5a-4bce-9345-5eb6f167457b.mov

After

282189199-da437bef-4b94-448f-85ee-78c41667eaf7.mp4

…infinite

boring-cyborg · 2023-11-10T00:06:48Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

potiuk · 2023-11-10T16:29:56Z

I think what would help to evaluate this change (not by me - I am pretty far from UI) is if you add some recorded videos before and after. It might be that what you see is somewhat result of your browser configuration and showing what you mean would be good to avoid wrong assumptions. Also showing effect of your change would greatly help in explaining what you really achieved with it.

caitpj · 2023-11-10T22:14:09Z

I think what would help to evaluate this change (not by me - I am pretty far from UI) is if you add some recorded videos before and after. It might be that what you see is somewhat result of your browser configuration and showing what you mean would be good to avoid wrong assumptions. Also showing effect of your change would greatly help in explaining what you really achieved with it.

Great idea, thank you. I've added a before and after video to the description above.

potiuk · 2023-11-11T07:56:09Z

I think what would help to evaluate this change (not by me - I am pretty far from UI) is if you add some recorded videos before and after. It might be that what you see is somewhat result of your browser configuration and showing what you mean would be good to avoid wrong assumptions. Also showing effect of your change would greatly help in explaining what you really achieved with it.

Great idea, thank you. I've added a before and after video to the description above.

Nice. I like it. I wonder what our UI team will say - but it looks rather cool.

pankajkoti · 2023-11-13T11:33:56Z

airflow/www/static/css/main.css

  .navbar-brand:hover .brand-logo-pinwheel {
    transform-origin: 17.66px 17.66px;
-    animation: pinSpin 1.5s linear;
+    animation: pinSpin 1.5s linear infinite;


Suggested change

animation: pinSpin 1.5s linear infinite;

animation: pinSpin 1.5s linear;

We wish to not have an infinite spin. There was an issue #34019 logged previously about this and hence we removed the infinite here #34020

I mention in the second paragraph of the description why I think removing infinite spin was not the right action to take away from issue #34019

Okay, thanks, sorry missed reading that 😐

jedcunningham · 2023-11-13T22:17:32Z

Maybe just move "Airflow" over a bit so that doesn't need to start moving too? In the "after" video it feels a bit jarring imo.

potiuk · 2023-11-13T22:34:28Z

Maybe just move "Airflow" over a bit so that doesn't need to start moving too? In the "after" video it feels a bit jarring imo.

Now we are talking design :D

potiuk · 2023-11-13T22:35:29Z

(and agree - when you move out the mouse, this is really disturbing when it suddenly goes back to start)

ryanahamilton · 2023-11-14T14:23:16Z

Figured I'd weigh in since I was the one who originally added this animation… The overlap was certainly something I noticed when implementing. I didn't increase the spacing between the word and pinwheel as I didn't want to alter the official brand logo, and I still don't believe we should. This was meant as a little Easter Egg interaction to add some fun to the UI. While I think this is a slick implementation, I think it is taking it a bit too far, and I'd advocate for removing the animation all together over making the animation more complex.

bbovenzi · 2023-11-14T17:38:57Z

I agree with Ryan, this was mainly an Easter Egg. I don't know if we want to mess with the logo too much. Can we double check everywhere else we use the Airflow SVG?

caitpj · 2023-11-14T19:45:09Z

I agree with Ryan, this was mainly an Easter Egg. I don't know if we want to mess with the logo too much. Can we double check everywhere else we use the Airflow SVG?

@bbovenzi I've had a look at the code base again and there is no other use of the logo as an SVG other than this one (in navbar.html). All the other uses of the logo use a png.

bbovenzi · 2023-11-15T16:08:03Z

I agree with Ryan, this was mainly an Easter Egg. I don't know if we want to mess with the logo too much. Can we double check everywhere else we use the Airflow SVG?

@bbovenzi I've had a look at the code base again and there is no other use of the logo as an SVG other than this one (in navbar.html). All the other uses of the logo use a png.

Ok, let's just change the spacing between the pinwheel and the text then. I agree with Jed, I'm not a fan of the text shifting on hover.

potiuk · 2023-11-15T16:41:48Z

I agree with Ryan, this was mainly an Easter Egg. I don't know if we want to mess with the logo too much. Can we double check everywhere else we use the Airflow SVG?

@bbovenzi I've had a look at the code base again and there is no other use of the logo as an SVG other than this one (in navbar.html). All the other uses of the logo use a png.

Ok, let's just change the spacing between the pinwheel and the text then. I agree with Jed, I'm not a fan of the text shifting on hover.

Damn... I knew I should never be a designer .

potiuk

REquest changes so that it is not merged accidentally

caitpj · 2023-11-15T20:57:16Z

How does something like this look? The downside to this option is it doesn't give as tight a connection between the logo pinwheel and the logo text, saying that, it doesn't disconnect it too much in my opinion.

Screen.Recording.2023-11-15.at.20.50.22.mov

jscheffl · 2023-11-20T19:44:43Z

I agree with Ryan, this was mainly an Easter Egg. I don't know if we want to mess with the logo too much. Can we double check everywhere else we use the Airflow SVG?

@bbovenzi I've had a look at the code base again and there is no other use of the logo as an SVG other than this one (in navbar.html). All the other uses of the logo use a png.

Ok, let's just change the spacing between the pinwheel and the text then. I agree with Jed, I'm not a fan of the text shifting on hover.

Damn... I knew I should never be a designer .

Besides lagging the "Easter Egg" history I very much like the animation and would prefer to keep it. I am not a 100% designed but I treated the text shift as rather "cool" :-D But seems there are different opinions.

The commit https://github.com/apache/airflow/assets/97813242/8f4f61d9-23ee-4553-a570-1ba758adb91a also looks good for me though.

eladkal · 2023-11-25T16:33:51Z

Besides lagging the "Easter Egg" history I very much like the animation and would prefer to keep it. I am not a 100% designed but I treated the text shift as rather "cool" :-D But seems there are different opinions.

I agree. + 1 from me.
I think we can accept this change

bbovenzi · 2023-11-28T22:40:55Z

airflow/www/static/css/main.css

+
+  .navbar-brand:hover .brand-logo-antidivider {
+    width: 0;
+    transition: width 0.5s;


Is the width still animating? I'm happy with the tiny bit more spacing all the time.

github-actions · 2024-01-15T00:12:54Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

add logo dynamic movement on hover and set pinwheel spinning time to …

90fe624

…infinite

caitpj requested review from ashb, bbovenzi, pierrejeambrun and ryanahamilton as code owners November 10, 2023 00:06

boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Nov 10, 2023

Merge branch 'main' into Stop-logo-clashing-when-pinwheel-animates

664b613

potiuk approved these changes Nov 11, 2023

View reviewed changes

remove unit to length units of size zero

8a1654f

pankajkoti reviewed Nov 13, 2023

View reviewed changes

potiuk requested changes Nov 15, 2023

View reviewed changes

bbovenzi reviewed Nov 28, 2023

View reviewed changes

github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 15, 2024

github-actions bot closed this Jan 22, 2024

	animation: pinSpin 1.5s linear infinite;
	animation: pinSpin 1.5s linear;

Stop logo clashing when pinwheel animates #35564

Stop logo clashing when pinwheel animates #35564

Uh oh!

Conversation

caitpj commented Nov 10, 2023 • edited by potiuk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boring-cyborg bot commented Nov 10, 2023

Uh oh!

potiuk commented Nov 10, 2023

Uh oh!

caitpj commented Nov 10, 2023

Uh oh!

potiuk commented Nov 11, 2023

Uh oh!

pankajkoti Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

caitpj Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

pankajkoti Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

jedcunningham commented Nov 13, 2023

Uh oh!

potiuk commented Nov 13, 2023

Uh oh!

potiuk commented Nov 13, 2023

Uh oh!

ryanahamilton commented Nov 14, 2023

Uh oh!

bbovenzi commented Nov 14, 2023

Uh oh!

caitpj commented Nov 14, 2023

Uh oh!

bbovenzi commented Nov 15, 2023

Uh oh!

potiuk commented Nov 15, 2023

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

caitpj commented Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jscheffl commented Nov 20, 2023

Uh oh!

eladkal commented Nov 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bbovenzi Nov 28, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

caitpj commented Nov 10, 2023 •

edited by potiuk

Loading

caitpj commented Nov 15, 2023 •

edited

Loading

eladkal commented Nov 25, 2023 •

edited

Loading