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

Use animated icon as status indicator #893

Merged
merged 1 commit into from
Mar 27, 2021

Conversation

fcollonval
Copy link
Member

Fixes #803

@github-actions
Copy link

github-actions bot commented Mar 6, 2021

Binder 👈 Launch a binder notebook on branch fcollonval/jupyterlab-git/fcollonval/issue803

@fcollonval
Copy link
Member Author

@mlucool @ianhi Happy to get your feedback on this one.

@mlucool
Copy link
Contributor

mlucool commented Mar 6, 2021

Thanks for the tag @fcollonval.

I really like the icon for refresh. While this is much better than the previous version, I still find the movement distracting. Can we invert this to only call attention to it when something is wrong or it's doing work triggered by a user? For example, automatically refreshing status is not work but committing a notebook is.

@krassowski
Copy link
Member

krassowski commented Mar 6, 2021

It might be still a bit distracting as discussed above, but aside from that I see a quirk with the icon jumping to the right when switching the panels (I assume it was not intended):

jitter

Edit: updated the GIF, but it does not catch the movement sometimes (sometimes it is too quick for the frame rate)

@krassowski
Copy link
Member

It seems that the issue that I am seeing is not with jupyterlab-git but with the core logs icon which is hidden (.jp-LogConsoleStatusItem) and has its classList populated with some delay after switching panels:

the-problem

@krassowski
Copy link
Member

I opened an issue upstream.

@fcollonval fcollonval force-pushed the fcollonval/issue803 branch from bba3c17 to 067672d Compare March 7, 2021 10:44
@fcollonval
Copy link
Member Author

I still find the movement distracting.

I switch to fill opacity animation to be less catchy.

Can we invert this to only call attention to it when something is wrong or it's doing work triggered by a user? For example, automatically refreshing status is not work but committing a notebook is.

The idea was to get the status bar reflecting the git actions and the pop-up to inform of the user actions progression. One of the point was to be able to see if for example refreshing was not finishing - this is by essence hard to automate as an arbitrary timeout changing the icon will be unable to take into account poor network for instance.

So I would rather keep the current logic but make it more discrete.

@fcollonval fcollonval marked this pull request as ready for review March 24, 2021 08:24
Fixes jupyterlab#803

Animate fill opacity instead of path

Remove class
@fcollonval fcollonval force-pushed the fcollonval/issue803 branch from f0061c7 to 36ca710 Compare March 27, 2021 16:24
@fcollonval fcollonval merged commit fa7eaa5 into jupyterlab:master Mar 27, 2021
@fcollonval fcollonval deleted the fcollonval/issue803 branch March 27, 2021 16:41
@fcollonval
Copy link
Member Author

@meeseeksdev backport to jlab-2

@lumberbot-app
Copy link

lumberbot-app bot commented Mar 27, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout jlab-2
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 fa7eaa5eb3bd29331d81ad4211c2c9e432dd6bd4
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #893: Use animated icon as status indicator'
  1. Push to a named branch :
git push YOURFORK jlab-2:auto-backport-of-pr-893-on-jlab-2
  1. Create a PR against branch jlab-2, I would have named this PR:

"Backport PR #893 on branch jlab-2"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UX: Bottom Bar Distracting
3 participants