refactor: split card ui experiment into two #4707
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
AmarTrebinjac
requested review from
rebelchris,
omBratteng,
ilasw,
capJavert,
sshanzel and
nensidosari
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Button Styling Misapplied to Label
The "Copy" QuaternaryButton incorrectly applies config.copyButtonClassName to both its className and labelClassName props. This class contains styles intended for the button container, not the label text. As a result, the button's container styling is misapplied, and the previous hover:bg-overlay-float-water effect is lost. The copyButtonClassName should be applied to the buttonClassName prop.
packages/shared/src/components/post/PostActions.tsx#L330-L331
apps/packages/shared/src/components/post/PostActions.tsx
Lines 330 to 331 in 2d8dbbf
Comment bugbot run to trigger another review on this PR
Was this report helpful? Give feedback by reacting with 👍 or 👎
| // Experiment configuration | ||
| const config = { | ||
| useNewExperience: colorExp || buttonExp, | ||
| showAwardAction: buttonExp, | ||
| copyButtonColor: colorExp ? ButtonColor.Water : ButtonColor.Cabbage, | ||
| copyButtonClassName: colorExp ? 'hover:text-text-link' : undefined, | ||
| background: colorExp ? 'bg-transparent' : 'bg-surface-float', | ||
| iconSize: buttonExp ? IconSize.Medium : IconSize.Medium, // Keep Medium for list view | ||
| }; |
There was a problem hiding this comment.
Can we abstract this maybe in a helper function/hook?
It being re-used makes for potentially issues on cleanup/additional tests
| // Experiment configuration | ||
| const config = { | ||
| showVoteButtonsInActions: buttonExp || colorExp, | ||
| showVoteButtonsInCard: !buttonExp && !colorExp, | ||
| copyButtonColor: colorExp ? ButtonColor.Water : ButtonColor.Cabbage, | ||
| copyButtonClassName: colorExp | ||
| ? 'group text-text-tertiary group-hover:text-text-link' | ||
| : 'btn-tertiary-cabbage', | ||
| cardBaseClassName: | ||
| 'flex !flex-row gap-2 hover:border-border-subtlest-tertiary', | ||
| }; |
| @@ -43,7 +43,7 @@ export const FreeformGrid = forwardRef(function SharePostCard( | |||
| const containerRef = useRef<HTMLDivElement>(); | |||
| const image = usePostImage(post); | |||
| const { title } = useSmartTitle(post); | |||
| const postUiExp = useFeature(featurePostUiImprovements); | |||
| const buttonExp = useFeature(featureCardUiButtons); | |||
There was a problem hiding this comment.
Refactored the name unintentionally?
Edit: ah, it is not. The name was just a bit confusing.
| @@ -50,7 +50,7 @@ export const ArticleGrid = forwardRef(function ArticleGrid( | |||
| }: PostCardProps, | |||
| ref: Ref<HTMLElement>, | |||
| ): ReactElement { | |||
| const postUiExp = useFeature(featurePostUiImprovements); | |||
| const buttonExp = useFeature(featureCardUiButtons); | |||
There was a problem hiding this comment.
Same here with the variable name.
| @@ -19,7 +19,7 @@ interface CollectionCardHeaderProps { | |||
| export const CollectionCardHeader = ({ | |||
| post, | |||
| }: CollectionCardHeaderProps): ReactElement => { | |||
| const postUiExp = useFeature(featurePostUiImprovements); | |||
| const buttonExp = useFeature(featureCardUiButtons); | |||
Changes
Split
post_ui_improvementsinto two experiments as requested here. New experiment iscard_ui_buttonsJust color
Buttons and color
Just buttons
List is the same with either experiment enabled, just adds extra award button
Post actions with experiment
Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Jira ticket
MI-942
Preview domain
https://mi-942.preview.app.daily.dev