Skip to content

Conversation

@benjaminleonard
Copy link
Contributor

@benjaminleonard benjaminleonard commented Apr 1, 2025

How does this 3 38 line CSS PR somehow add sound effects to all the buttons? Beats me.

@vercel
Copy link

vercel bot commented Apr 1, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Apr 16, 2025 3:52pm

@david-crespo
Copy link
Collaborator

david-crespo commented Apr 1, 2025

Very cool. I like it. Not sure how I feel about the dropdown buttons getting it, feels slightly weird. Especially how the menu itself appears to move with it (that may be the primary reason it feels weird). Also the docs popover button is missing it.

2025-04-01-button-move-dropdowns.mp4

@benjaminleonard
Copy link
Contributor Author

I tried without and they felt a little left out, especially since they look like buttons. Pushed a little fix that applies it to a child of the button – that way the trigger doesn't move with it. How does that feel?

@david-crespo
Copy link
Collaborator

Definitely better without moving the menu. Another little issue: when I tap on my trackpad, sometimes it's fast enough to not move perceptibly, but other times I get this twitch. Interestingly the video doesn't really capture it, maybe because it's double size? It looks more jerky IRL.

2025-04-02-quick-click-2.mp4

@benjaminleonard
Copy link
Contributor Author

Definitely better without moving the menu. Another little issue: when I tap on my trackpad, sometimes it's fast enough to not move perceptibly, but other times I get this twitch. Interestingly the video doesn't really capture it, maybe because it's double size? It looks more jerky IRL.

We're relying on the browser active – so there's probably only so much we can influence that. Does it happen on regular buttons too as well as dropdowns. That way we can rule out it being applied to a div inside the button instead of the button itself.

If that's not the issue, we could potentially add a transition. It's a 1px translate, but whatever animation shenanigans might make sure it sticks around for longer than the life of the active property.

@david-crespo
Copy link
Collaborator

This works pretty well to do the opposite — make sure it doesn't move in case of super short presses. I don't think we can make it work the other with just :active.

.active-clicked {
   @apply transition-transform duration-[50ms] ease-[cubic-bezier(0.65,0,0.35,1)];
}

@benjaminleonard
Copy link
Contributor Author

That felt a little weirder to me, like it was occasionally completely broken.

This might be a bit of a browser limitation. I tried a combination of delays / durations / animations to no real benefit. The problem I think is that the application of active and removal is too quick to transition.

We could in theory do with js but that kills the simplicity of it, and probably unnecessary overhead. You might just be an exceptionally quick clicker.

@david-crespo
Copy link
Collaborator

I think it's actually the way the tap clicks work on my MBP trackpad. When I tap to click rather than fully pressing, it's not about how fast I do it — it does not appear possible to do it slowly enough to see the full animation. If I tap slower it stops being a click at all.

<PopoverButton title={title}>
<div className={cn(buttonStyle({ size: 'sm', variant: 'ghost' }), 'w-8')}>
<Info16Icon aria-hidden className="shrink-0" />
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one doesn't work, not sure why. When I move buttonStyle to the PopoverButton, I can get it to move when I check :active in the dev tools, but that doesn't happen when I actually click it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works for me, can you still reproduce?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Turns out it's only broken in Firefox. I think we can live with that.

@david-crespo david-crespo merged commit f493cb3 into main Apr 16, 2025
13 checks passed
@david-crespo david-crespo deleted the click-clack-button branch April 16, 2025 20:41
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request May 7, 2025
oxidecomputer/console@99a1f00...46e244f

* [46e244fd](oxidecomputer/console@46e244fd) oxidecomputer/console#2818
* [9811a9c7](oxidecomputer/console@9811a9c7) element=null on root redirect route to silence warnings
* [b8e9f385](oxidecomputer/console@b8e9f385) oxidecomputer/console#2821
* [f097ec96](oxidecomputer/console@f097ec96) oxidecomputer/console#2819
* [585fe913](oxidecomputer/console@585fe913) oxidecomputer/console#2815
* [973d332a](oxidecomputer/console@973d332a) oxidecomputer/console#2817
* [ad61a081](oxidecomputer/console@ad61a081) oxidecomputer/console#2816
* [239e34b9](oxidecomputer/console@239e34b9) oxidecomputer/console#2806
* [376f172f](oxidecomputer/console@376f172f) oxidecomputer/console#2805
* [5c7c8b5b](oxidecomputer/console@5c7c8b5b) oxidecomputer/console#2746
* [1d8c3b76](oxidecomputer/console@1d8c3b76) bump playwright, use more cores to run e2es locally
* [f493cb35](oxidecomputer/console@f493cb35) oxidecomputer/console#2772
* [819b99a6](oxidecomputer/console@819b99a6) chore: type error after RR 7.5. this is why we make PRs
* [c1134cff](oxidecomputer/console@c1134cff) chore: react-router 7.5.0
* [c1f0d78b](oxidecomputer/console@c1f0d78b) chore: vite 6.3.0 + vitest 3.1.1
* [79c6bc53](oxidecomputer/console@79c6bc53) oxidecomputer/console#2803
* [f454f5ee](oxidecomputer/console@f454f5ee) oxidecomputer/console#2804
* [ccfa5a58](oxidecomputer/console@ccfa5a58) fix deploy-dogfood script when used with a tag
* [6196fa48](oxidecomputer/console@6196fa48) invalidate anti-affinity group member lists on instance action success
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request May 7, 2025
oxidecomputer/console@99a1f00...46e244f

* [46e244fd](oxidecomputer/console@46e244fd)
oxidecomputer/console#2818
* [9811a9c7](oxidecomputer/console@9811a9c7)
element=null on root redirect route to silence warnings
* [b8e9f385](oxidecomputer/console@b8e9f385)
oxidecomputer/console#2821
* [f097ec96](oxidecomputer/console@f097ec96)
oxidecomputer/console#2819
* [585fe913](oxidecomputer/console@585fe913)
oxidecomputer/console#2815
* [973d332a](oxidecomputer/console@973d332a)
oxidecomputer/console#2817
* [ad61a081](oxidecomputer/console@ad61a081)
oxidecomputer/console#2816
* [239e34b9](oxidecomputer/console@239e34b9)
oxidecomputer/console#2806
* [376f172f](oxidecomputer/console@376f172f)
oxidecomputer/console#2805
* [5c7c8b5b](oxidecomputer/console@5c7c8b5b)
oxidecomputer/console#2746
* [1d8c3b76](oxidecomputer/console@1d8c3b76)
bump playwright, use more cores to run e2es locally
* [f493cb35](oxidecomputer/console@f493cb35)
oxidecomputer/console#2772
* [819b99a6](oxidecomputer/console@819b99a6)
chore: type error after RR 7.5. this is why we make PRs
* [c1134cff](oxidecomputer/console@c1134cff)
chore: react-router 7.5.0
* [c1f0d78b](oxidecomputer/console@c1f0d78b)
chore: vite 6.3.0 + vitest 3.1.1
* [79c6bc53](oxidecomputer/console@79c6bc53)
oxidecomputer/console#2803
* [f454f5ee](oxidecomputer/console@f454f5ee)
oxidecomputer/console#2804
* [ccfa5a58](oxidecomputer/console@ccfa5a58)
fix deploy-dogfood script when used with a tag
* [6196fa48](oxidecomputer/console@6196fa48)
invalidate anti-affinity group member lists on instance action success
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