- Notifications
You must be signed in to change notification settings - Fork1k
chore(site): convert more components from Emotion to TailwindCSS#19719
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
302dd27d7de1b33ab9a6c2854960619884baa5e6430bf2a86c0dd6799b59d61f96516e07bffaaf498a9a7bef950d7318a359f551175ec0110b4477e695df9742908bda9258506e11e22f06e42abea2627cf3da37097e42edd645b6fff224ef62604666f0e3d5bdf7d1File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| declare module"react"{ | ||
| interfaceCSSProperties{ | ||
| [key: `--${string}`]:string|number|undefined; | ||
| } | ||
| } | ||
| export{}; | ||
Comment on lines +1 to +7 MemberAuthor
| ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -52,8 +52,8 @@ export const SelectFilter: FC<SelectFilterProps> = ({ | ||
| <SelectMenuTrigger> | ||
| <SelectMenuButton | ||
| startIcon={selectedOption?.startIcon} | ||
| className="shrink-0 grow" | ||
| style={{ flexBasis: width }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. yeah, this always should've been a | ||
| aria-label={label} | ||
| > | ||
| {selectedOption?.label ?? placeholder} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -506,7 +506,7 @@ export const MultiSelectCombobox = forwardRef< | ||
| <Badge | ||
| key={option.value} | ||
| className={cn( | ||
| "data-[disabled]:bg-content-disabled data-[disabled]:text-surface-tertiary data-[disabled]:hover:bg-content-disabled", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. We had a missing space before, so two of the classes would just never trigger There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I love Tailwind | ||
| "data-[fixed]:bg-content-disabled data-[fixed]:text-surface-tertiary data-[fixed]:hover:bg-surface-secondary", | ||
| badgeClassName, | ||
| )} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,46 @@ | ||
| importtype{Meta,StoryObj}from"@storybook/react-vite"; | ||
| import{Search,SearchEmpty,SearchInput}from"./Search"; | ||
| constmeta:Meta<typeofSearchInput>={ | ||
| title:"components/Search", | ||
| component:SearchInput, | ||
Comment on lines -8 to -12 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This was making it so that every time you tried to mount | ||
| }; | ||
| exportdefaultmeta; | ||
| typeStory=StoryObj<typeofSearchInput>; | ||
| exportconstExample:Story={ | ||
| render:(props)=>( | ||
| <Search> | ||
| <SearchInput{...props}/> | ||
| </Search> | ||
| ), | ||
| }; | ||
| exportconstWithCustomPlaceholder:Story={ | ||
| args:{ | ||
| label:"uwu", | ||
| placeholder:"uwu", | ||
| }, | ||
| render:(props)=>( | ||
| <Search> | ||
| <SearchInput{...props}/> | ||
| </Search> | ||
| ), | ||
| }; | ||
| exportconstWithSearchEmpty:Story={ | ||
| args:{ | ||
| label:"I crave the certainty of steel", | ||
| placeholder:"Alas, I am empty", | ||
| }, | ||
| render:(props)=>( | ||
| <divclassName="flex flex-col gap-2"> | ||
| <Search> | ||
| <SearchInput{...props}/> | ||
| </Search> | ||
| <SearchEmpty/> | ||
| </div> | ||
| ), | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,9 @@ | ||
| import { SearchIcon } from "lucide-react"; | ||
| import type { FC, HTMLAttributes, InputHTMLAttributes, Ref } from "react"; | ||
| import { cn } from "utils/cn"; | ||
| interface SearchProps extendsHTMLAttributes<HTMLDivElement> { | ||
| ref?: Ref<HTMLDivElement>; | ||
| } | ||
| /** | ||
| @@ -18,100 +15,63 @@ interface SearchProps extends Omit<BoxProps, "ref"> { | ||
| * </Search> | ||
| * ``` | ||
| */ | ||
| export const Search: FC<SearchProps> = ({ | ||
| children, | ||
| ref, | ||
| className, | ||
| ...props | ||
| }) => { | ||
| return ( | ||
| <div | ||
| ref={ref} | ||
| {...props} | ||
| className={cn( | ||
| "flex items-center h-10 pl-4 border-0 border-b border-solid border-border", | ||
| className, | ||
| )} | ||
| > | ||
| <SearchIcon className="size-icon-xs text-sm text-content-secondary" /> | ||
| {children} | ||
| </div> | ||
| ); | ||
| }; | ||
| type SearchInputProps = InputHTMLAttributes<HTMLInputElement> & { | ||
| label?: string; | ||
| ref?: Ref<HTMLInputElement>; | ||
| }; | ||
| export const SearchInput: FC<SearchInputProps> = ({ | ||
| label, | ||
| ref, | ||
| id, | ||
| ...inputProps | ||
| }) => { | ||
| return ( | ||
| <> | ||
| <labelclassName="sr-only"htmlFor={id}> | ||
| {label} | ||
| </label> | ||
| <input | ||
| ref={ref} | ||
| id={id} | ||
MemberAuthor
| ||
| tabIndex={0} | ||
| type="text" | ||
| placeholder="Search..." | ||
| className="text-inherit h-full border-0 bg-transparent grow basis-0 outline-none pl-4 placeholder:text-content-secondary" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I don't know if this CSS is 100% best practices, but we have a bunch of I think that most of the time, we just wanted | ||
| {...inputProps} | ||
| /> | ||
| </> | ||
| ); | ||
| }; | ||
| export const SearchEmpty: FC<HTMLAttributes<HTMLDivElement>> = ({ | ||
| children = "Not found", | ||
| ...props | ||
| }) => { | ||
| return ( | ||
| <divclassName="text-sm text-content-secondary text-center py-2" {...props}> | ||
| {children} | ||
| </div> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This was only ever used for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. very nice. this should've been a | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,51 +1,49 @@ | ||||||
| import IconButton from "@mui/material/IconButton"; | ||||||
| import InputAdornment from "@mui/material/InputAdornment"; | ||||||
| import TextField, { type TextFieldProps } from "@mui/material/TextField"; | ||||||
| import Tooltip from "@mui/material/Tooltip"; | ||||||
| import{ useEffectEvent }from "hooks/hookPolyfills"; | ||||||
| import { SearchIcon, XIcon } from "lucide-react"; | ||||||
| import { type FC,useLayoutEffect, useRef } from "react"; | ||||||
| export type SearchFieldProps = Omit<TextFieldProps, "onChange"> & { | ||||||
| onChange: (query: string) => void; | ||||||
| autoFocus?: boolean; | ||||||
| }; | ||||||
| export const SearchField: FC<SearchFieldProps> = ({ | ||||||
| InputProps, | ||||||
| onChange, | ||||||
| value = "", | ||||||
| autoFocus = false, | ||||||
| ...textFieldProps | ||||||
| }) => { | ||||||
| // MUI's autoFocus behavior is wonky. If you set autoFocus=true, the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. it seems to just pass it down to the HTML. but the code is hard to follow, so it might be doing something else. if it causes issues tho I guess it's all the same. 🤷♀️ | ||||||
| // component will keep getting focus on every single render, even if there | ||||||
| // are other input elements on screen. We want this to be one-time logic | ||||||
| const inputRef = useRef<HTMLInputElement | null>(null); | ||||||
| const focusOnMount = useEffectEvent((): void => { | ||||||
| if (autoFocus) { | ||||||
| inputRef.current?.focus(); | ||||||
| } | ||||||
| }); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This wasn't hard to fix, but it was a nasty bug, and I think the only reason we haven't had a problem is because we never had any components with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. was the bug before just that this is missing the deps list? Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yeah, this was running on each render because there was no list | ||||||
| useLayoutEffect(() => { | ||||||
| focusOnMount(); | ||||||
| }, [focusOnMount]); | ||||||
| return ( | ||||||
| <TextField | ||||||
| inputRef={inputRef} | ||||||
| // Specifying min width so that the text box can't shrink so much | ||||||
| // that it becomes un-clickable as we add more filter controls | ||||||
| className="min-w-[280px]" | ||||||
| size="small" | ||||||
| value={value} | ||||||
| onChange={(e) => onChange(e.target.value)} | ||||||
| InputProps={{ | ||||||
| startAdornment: ( | ||||||
| <InputAdornment position="start"> | ||||||
| <SearchIcon className="size-icon-xs text-content-secondary" /> | ||||||
| </InputAdornment> | ||||||
| ), | ||||||
| endAdornment: value !== "" && ( | ||||||
| @@ -58,7 +56,7 @@ export const SearchField: FC<SearchFieldProps> = ({ | ||||||
| }} | ||||||
| > | ||||||
| <XIcon className="size-icon-xs" /> | ||||||
| <spanclassName="sr-only">Clear search</span> | ||||||
| </IconButton> | ||||||
| </Tooltip> | ||||||
| </InputAdornment> | ||||||
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.