revert: restore the accordion trigger's hover underline - #1145
Merged
Conversation
This reverts commit 30b9478.
Completes the revert. The trigger is now byte-identical to what it was before either of the two hover changes, back on shadcn's underline. The reasoning I gave for replacing it (an underline reads as a link, and this is a disclosure control) was not wrong on its own terms, but it was not a problem anyone had, and shadcn parity is the stronger default for a kit whose whole promise is that shadcn knowledge transfers.
vivek7405
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #1137 and the accordion half of #1126, at the owner's direction.
The trigger's hover was changed twice recently, both times by me, and neither landed. This puts it back to where it started, on shadcn's underline. The file is byte-identical to its state before either change (
git diff 5e9be0b1 HEAD -- accordion.tsis empty).hover:underlinehover:text-foreground/80hover:bg-accent hover:text-accent-foregroundThe argument I made for replacing the underline (it reads as a link affordance, and a
<summary>is a disclosure control rather than a navigation) was not wrong in itself. It just was not a problem anyone had, and it traded away shadcn parity, which is the stronger default for a kit whose whole premise is that shadcn knowledge transfers directly.Nothing shipped either way:
@webjsdev/uiis at 0.3.10 on npm and neither change was ever released.Test plan
@webjsdev/ui187 pass.Worth recording: no test ever asserted the hover treatment, which is why it moved three times without friction. Not adding one here, because pinning a class string that is deliberately inherited from shadcn would just make the next sync noisier. The real guard is the parity rule in
packages/ui/AGENTS.mdinvariant 5.