Skip to content

chore: remove some usage of useClassName #19346

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

Merged
merged 12 commits into from
Aug 14, 2025
Merged

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Aug 13, 2025

This hook is major tech debt. It's pretty hacky way to get emotion to give you a stable class name, which was more necessary when it was our primary method of styling. Now that we're migrating to Tailwind we should get rid of all usage of this hook and eventually remove it.

@aslilac aslilac requested a review from brettkolodny August 13, 2025 23:13
switch (variant) {
case "error":
return theme.palette.error.main;
return "border-highlight-red";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double checking, did border-border-destructive not look right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they're basically the same color

)}
</IconButton>
</Tooltip>
</div>
<div css={{ fontWeight: 500, marginTop: 16 }}>
<div className="text-medium mt-4">
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be font-medium as well as the ones below

@aslilac aslilac requested a review from brettkolodny August 14, 2025 20:46
@aslilac aslilac merged commit bb0e407 into main Aug 14, 2025
32 checks passed
@aslilac aslilac deleted the lilac/use-class-name-pt-1 branch August 14, 2025 21:53
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants