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

Add animations to the configuration UI #844

Merged
merged 11 commits into from Jan 27, 2021

Conversation

hatched
Copy link
Contributor

@hatched hatched commented Jan 25, 2021

Done

Add animations and fades to give the config some life.

Blocked on #843 landing, once it's landed then this can be reviewed and qa'd

QA

  • Use the demo link
  • Play around with the config and see the animations work, the list of the addressed items are in the issue.

Details

https://github.com/canonical-web-and-design/juju-squad/issues/1601

@webteam-app
Copy link

Demo starting at https://jaas-dashboard-844.demos.haus

@spencerbygraves
Copy link

Thanks for looking at this @hatched, a couple of things I noticed...

  • For consistency, are we using the same animation values as elsewhere in the dashboard? For example, the fade on the table row hover blue looks to be slightly different to the config blue
  • When I focus a field, only the first description has a fade in. When I focus the others there is no fade in/out

Perhaps unrelated to this PR...

  • When I select an app, sometimes the reset button and use default message is already there, others not until I focus a field

image

  • The reset all button doesn't work after I've edited some values

@anthonydillon
Copy link
Contributor

@hatched can you adjust the motion duration. It seems a little slow leading to a laggy feel. Maybe try halving the default. We have some standard durations here: https://vanillaframework.io/docs/settings/animation-settings

@hatched
Copy link
Contributor Author

hatched commented Jan 26, 2021

These animations are all using the defined Vanilla speeds, they are all set to brisk. It's easy to update to another but this was the speed that felt good to me :)

When I select an app, sometimes the reset button and use default message is already there, others not until I focus a field

This is it working correctly. If the configuration value has been changed from the default already then these will already be there when you view the config.

When I focus a field, only the first description has a fade in. When I focus the others there is no fade in/out

Yes this is a limitation of how React works - We can come up with a resolution but after doing it as a hack I found that the snap to other description felt better.

Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

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

LGTM, few comments in the code but no blockers

@hatched
Copy link
Contributor Author

hatched commented Jan 27, 2021

Thanks for the reviews!

@hatched hatched merged commit ff2158c into canonical:master Jan 27, 2021
@hatched hatched deleted the animate-configuration branch January 27, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants