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

shell: Animate host switcher caret #19815

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Conversation

MeastroZI
Copy link
Contributor

@MeastroZI MeastroZI commented Jan 4, 2024

"Introducing a minor update:

Changes: Improved caret transition in the document, accessible through .

View the modifications :

Screencast.2024-01-05.19.00.46.mp4

Previous version :

Screencast.2024-01-05.18.44.52.mp4

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks for your first contribution, exciting! This still needs some work though, I left some comments below.

pkg/shell/hosts.jsx Outdated Show resolved Hide resolved
pkg/shell/hosts.jsx Outdated Show resolved Hide resolved
pkg/shell/nav.scss Outdated Show resolved Hide resolved
pkg/shell/nav.scss Outdated Show resolved Hide resolved
pkg/shell/nav.scss Show resolved Hide resolved
pkg/shell/nav.scss Outdated Show resolved Hide resolved
@martinpitt
Copy link
Member

@garrett I'm afraid this isn't really my area of expertise. Could you please have a look? The change looks technically plausible now. Thanks @MeastroZI !

garrett
garrett previously approved these changes Jan 22, 2024
Copy link
Member

@garrett garrett left a comment

Choose a reason for hiding this comment

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

Thanks!

We already had caret animation like this in other parts of Cockpit (expandable tables, for example). Thanks for adding it to the dropdown of the host switcher too!

@martinpitt martinpitt changed the title Caret transition shell: Animate host switcher caret Jan 23, 2024
@martinpitt
Copy link
Member

Thanks @MeastroZI ! I rebased your branch, fixed the commit message and the trailing space in the CSS, and drive this to completion. I also tested it and it works nicely now. Cheers! 🎉

@martinpitt
Copy link
Member

This pixel test failure points out a problem, the caret isn't yet animated to close again. Most likely we'll just need to wait a bit, checking.

@martinpitt
Copy link
Member

Ah, it is wrong because the caret is really the wrong way around -- it should point down when closed, and point up when opened. I flipped it around.

The new caret is also a bit bigger, which is fine. updating the pixel tests accordingly.

@martinpitt martinpitt merged commit 9eeb2b0 into cockpit-project:main Jan 23, 2024
49 of 91 checks passed
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.

3 participants