-
Notifications
You must be signed in to change notification settings - Fork 152
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
feat(i18n): Setup react-i18next #9797
Changes from 4 commits
24e2f21
e4f424e
b5a4584
1b51258
501ee36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,6 @@ import * as Schema from "v2/System/Analytics/Schema" | |
import { SystemQueryRenderer } from "v2/System/Relay/SystemQueryRenderer" | ||
import { | ||
FirstSuggestionItem, | ||
PLACEHOLDER, | ||
PLACEHOLDER_XS, | ||
SuggestionItem, | ||
} from "v2/Components/Search/Suggestions/SuggestionItem" | ||
import { Router } from "found" | ||
|
@@ -28,10 +26,11 @@ import { useDidMount } from "v2/Utils/Hooks/useDidMount" | |
import createLogger from "v2/Utils/logger" | ||
import { Media } from "v2/Utils/Responsive" | ||
import { SearchInputContainer } from "./SearchInputContainer" | ||
import { withTranslation, WithTranslation } from "react-i18next" | ||
|
||
const logger = createLogger("Components/Search/SearchBar") | ||
|
||
export interface Props extends SystemContextProps { | ||
export interface Props extends SystemContextProps, WithTranslation { | ||
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. why would we do this here this way, and not with the same 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. oh i see.. this is a class component. even though it makes sense, I would much prefer it if we used something like https://github.com/artsy/eigen/blob/26a98c408c223efefbb0b8309448c3bb79256ab3/src/app/Scenes/Search/components/CityGuideCTA.tsx#L12-L13 (docs here at the bottom https://github.com/artsy/eigen/blob/b4484db7fb7ddfbad4e903c993628bbd90a909b3/docs/palette_v3_migration_guide.md) 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. Thanks @pvinis !! ❤️ As this is not part of an actual project, I don't think I'll have time to focus a lot on these changes before Wednesday. Do you think it would make sense to remove 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. If you think the way I suggest is a good way to do this change for the file here, I'm happy to help or even make a PR on top of this one with the change. If you think we need more discussion on this, I'll gladly greenlight this 👍. 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 completely agree that your suggestion would be much cleaner, if it's possible to open a PR on top of this one, that would be great. I just don't want to put any pressure on you either, I'm already super grateful for the careful code review. ❤️ 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'll take a quick stab at it. |
||
relay: RelayRefetchProp | ||
router?: Router | ||
viewer: SearchBar_viewer | ||
|
@@ -373,7 +372,7 @@ export class SearchBar extends Component<Props, State> { | |
|
||
renderAutosuggestComponent({ xs }) { | ||
const { term } = this.state | ||
const { viewer } = this.props | ||
const { viewer, t } = this.props | ||
|
||
const inputProps = { | ||
"aria-label": "Search Artsy", | ||
|
@@ -390,7 +389,7 @@ export class SearchBar extends Component<Props, State> { | |
event.preventDefault() | ||
} | ||
}, | ||
placeholder: xs ? PLACEHOLDER_XS : PLACEHOLDER, | ||
placeholder: xs ? t`navbar.searchArtsy` : t`navbar.searchBy`, | ||
value: term, | ||
} | ||
|
||
|
@@ -465,7 +464,7 @@ export class SearchBar extends Component<Props, State> { | |
} | ||
|
||
export const SearchBarRefetchContainer = createRefetchContainer( | ||
withSystemContext(SearchBar), | ||
withSystemContext(withTranslation()(SearchBar)), | ||
{ | ||
viewer: graphql` | ||
fragment SearchBar_viewer on Viewer | ||
|
@@ -509,35 +508,43 @@ export const SearchBarRefetchContainer = createRefetchContainer( | |
* Displays during SSR render, but once mounted is swapped out with | ||
* QueryRenderer below. | ||
*/ | ||
const StaticSearchContainer: React.FC<{ searchQuery: string } & BoxProps> = ({ | ||
searchQuery, | ||
...rest | ||
}) => { | ||
const StaticSearchContainer: React.FC< | ||
{ searchQuery: string } & BoxProps & WithTranslation | ||
> = ({ searchQuery, t, ...rest }) => { | ||
return ( | ||
<> | ||
<Box display={["block", "none"]} {...rest}> | ||
<SearchInputContainer | ||
placeholder={searchQuery || PLACEHOLDER_XS} | ||
placeholder={searchQuery || t`navbar.searchArtsy`} | ||
defaultValue={searchQuery} | ||
/> | ||
</Box> | ||
|
||
<Box display={["none", "block"]} {...rest}> | ||
<SearchInputContainer | ||
placeholder={searchQuery || PLACEHOLDER} | ||
placeholder={searchQuery || t`navbar.searchBy`} | ||
defaultValue={searchQuery} | ||
/> | ||
</Box> | ||
</> | ||
) | ||
} | ||
|
||
const WithTranslationStaticSearchContainer = withTranslation()( | ||
StaticSearchContainer | ||
) | ||
|
||
export const SearchBarQueryRenderer: React.FC<BoxProps> = props => { | ||
const { relayEnvironment, searchQuery = "" } = useContext(SystemContext) | ||
const isMounted = useDidMount(typeof window !== "undefined") | ||
|
||
if (!isMounted) { | ||
return <StaticSearchContainer searchQuery={searchQuery} {...props} /> | ||
return ( | ||
<WithTranslationStaticSearchContainer | ||
searchQuery={searchQuery} | ||
{...props} | ||
/> | ||
) | ||
} | ||
|
||
return ( | ||
|
@@ -561,7 +568,9 @@ export const SearchBarQueryRenderer: React.FC<BoxProps> = props => { | |
// from within the NavBar (it's not a part of any app) we need to lean | ||
// on styled-system for showing / hiding depending upon breakpoint. | ||
} else { | ||
return <StaticSearchContainer searchQuery={searchQuery} /> | ||
return ( | ||
<WithTranslationStaticSearchContainer searchQuery={searchQuery} /> | ||
) | ||
} | ||
}} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,9 +84,6 @@ const SuggestionItemLink = styled(RouterLink).attrs({ | |
min-height: 60px; | ||
` | ||
|
||
export const PLACEHOLDER = "Search by artist, gallery, style, theme, tag, etc." | ||
export const PLACEHOLDER_XS = "Search Artsy" | ||
Comment on lines
-87
to
-88
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. These weren't being used anywhere besides inside the |
||
|
||
const DefaultSuggestion: React.FC<SuggestionItemProps> = ({ | ||
display, | ||
label, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice