feat: enabling i18n feature for smaller screens #3556
Conversation
devilkiller-ag
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3556 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 732 732
=========================================
Hits 732 732 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
utils/getStatic.ts (1)
Line range hint
32-36: Improve type safety and error handlingThe function uses loose typing and could benefit from better error handling.
-export async function getI18nProps(ctx: any, ns = ['common']) { +interface I18nContext { + params?: { + lang?: string; + }; +} + +export async function getI18nProps(ctx: I18nContext, ns = ['common']) { const locale = ctx?.params?.lang ? ctx.params.lang : 'english'; + + if (!i18nextConfig.i18n.locales.includes(locale)) { + console.warn(`Invalid locale: ${locale}, falling back to english`); + return serverSideTranslations('english', ns); + } + const props = { ...(await serverSideTranslations(locale, ns)) };
🧹 Nitpick comments (7)
next-i18next.config.js (2)
3-4: Consider using standard ISO 639-1 language codesUsing full language names ('english', 'deutsch') instead of standard ISO 639-1 codes ('en', 'de') deviates from web standards. This could impact:
- SEO optimization
- Browser language detection
- Accessibility tools
- Integration with third-party i18n tools
Consider keeping ISO codes internally while displaying full names in the UI.
🧰 Tools
🪛 eslint
[error] 3-3: Delete
··(prettier/prettier)
[error] 4-4: Replace
······defaultLocale·with····defaultLocale(prettier/prettier)
3-9: Fix formatting issuesThere are inconsistent indentation and spacing issues in the configuration.
module.exports = { i18n: { - locales: ['english', 'deutsch'], - defaultLocale : 'english', - namespaces: ['landing-page', 'common', 'tools'], - defaultNamespace: 'landing-page', - react: { useSuspense: false },// this line - }, + locales: ['english', 'deutsch'], + defaultLocale: 'english', + namespaces: ['landing-page', 'common', 'tools'], + defaultNamespace: 'landing-page', + react: { useSuspense: false } + } };🧰 Tools
🪛 eslint
[error] 3-3: Delete
··(prettier/prettier)
[error] 4-4: Replace
······defaultLocale·with····defaultLocale(prettier/prettier)
[error] 5-5: Delete
··(prettier/prettier)
[error] 6-6: Delete
··(prettier/prettier)
[error] 7-7: Replace
··react:·{·useSuspense:·false·},withreact:·{·useSuspense:·false·}·(prettier/prettier)
[error] 8-8: Replace
··},with}(prettier/prettier)
utils/i18n.ts (1)
20-21: Enhance type safety for language identifiersConsider using TypeScript enums or const assertions for better type safety and autocompletion.
-export const languages = ['english', 'deutsch']; -export const defaultLanguage = 'english'; +export const languages = ['english', 'deutsch'] as const; +export type SupportedLanguage = typeof languages[number]; +export const defaultLanguage: SupportedLanguage = 'english';components/icons/Language.tsx (2)
4-6: Enhance component documentationThe JSDoc comment could be more descriptive about the component's purpose and usage.
/** - * @description Icons for asyncapi website + * @description Language icon component used in the language selector + * @param {Object} props - Component props + * @param {string} [props.className=''] - Additional CSS classes to apply + * @returns {JSX.Element} Language icon SVG */
7-23: Add accessibility attributes to SVGThe SVG icon should include ARIA attributes for better accessibility.
-export default function IconLanguage({ className = '' }) { +export default function IconLanguage({ className = '' }: { className?: string }) { return ( <svg xmlns='http://www.w3.org/2000/svg' fill='none' viewBox='0 0 24 24' strokeWidth={1.5} stroke='currentColor' + role="img" + aria-label="Language selector" className={`size-5 ${className}`} >components/languageSelector/LanguageSelect.tsx (1)
16-35: LGTM! Well-structured language selector implementation.The changes effectively integrate the language icon while maintaining accessibility and proper spacing. The use of relative positioning and flex layout ensures proper alignment.
A minor suggestion to enhance accessibility:
- <select + <select + aria-label="Select language" data-testid='Select-form'components/navigation/NavBar.tsx (1)
236-236: Reduce duplication in language name formatting.The language name capitalization logic is repeated in multiple places. Consider extracting it to a utility function.
+ // Add to utils/i18n.ts + export const formatLanguageName = (lang: string): string => + lang ? lang.charAt(0).toUpperCase() + lang.slice(1) : 'English'; - selected={i18n.language ? i18n.language.charAt(0).toUpperCase() + i18n.language.slice(1) : 'English'} + selected={formatLanguageName(i18n.language)} - currentLanguage={i18n.language ? i18n.language.charAt(0).toUpperCase() + i18n.language.slice(1) : 'English'} + currentLanguage={formatLanguageName(i18n.language)}Also applies to: 254-254
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/icons/Icons.mdx(2 hunks)components/icons/Language.tsx(1 hunks)components/languageSelector/LanguageSelect.tsx(2 hunks)components/navigation/MobileNavMenu.tsx(4 hunks)components/navigation/NavBar.tsx(5 hunks)next-i18next.config.js(1 hunks)utils/getStatic.ts(1 hunks)utils/i18n.ts(1 hunks)
🧰 Additional context used
🪛 eslint
next-i18next.config.js
[error] 3-3: Delete ··
(prettier/prettier)
[error] 4-4: Replace ······defaultLocale· with ····defaultLocale
(prettier/prettier)
[error] 5-5: Delete ··
(prettier/prettier)
[error] 6-6: Delete ··
(prettier/prettier)
[error] 7-7: Replace ··react:·{·useSuspense:·false·}, with react:·{·useSuspense:·false·}·
(prettier/prettier)
[error] 8-8: Replace ··}, with }
(prettier/prettier)
🔇 Additional comments (4)
utils/i18n.ts (1)
8-17: Validate path consistencyThe i18nPaths contain relative URLs. Consider:
- Validating paths against your router configuration
- Using path constants to prevent typos
- Adding comments for path purpose
components/navigation/MobileNavMenu.tsx (1)
23-25: LGTM! Well-typed props interface.The props interface clearly defines the required types for language handling.
components/navigation/NavBar.tsx (1)
Line range hint
46-62: LGTM! Clear documentation and implementation of getUniqueLangs.The function properly handles language name capitalization and provides a sensible default.
components/icons/Icons.mdx (1)
33-33: LGTM! Proper integration of language icon.The IconLanguage component is correctly imported and documented in the icon gallery, following the established pattern.
Also applies to: 240-242
| <div className='space-y-2 px-5 py-2' onClick={() => showMenu('language')}> | ||
| <div className='grid gap-4'> | ||
| <div> | ||
| <h4 className='mb-4 flex justify-between font-medium text-gray-800'> | ||
| <a className='flex cursor-pointer items-center gap-x-2'> | ||
| Language <IconLanguage /> | ||
| </a> | ||
| <NavItemDropdown /> | ||
| </h4> | ||
| {open === 'language' && | ||
| uniqueLangs.map((lang) => ( | ||
| <button | ||
| key={lang.key} | ||
| onClick={() => changeLanguage(lang.value.toLowerCase(), true)} | ||
| className={`mb-4 ml-2 block w-full rounded-lg py-1 text-start text-sm font-medium leading-6 text-gray-700 transition duration-150 ease-in-out hover:bg-gray-50 ${currentLanguage.toLowerCase() === lang.text.toLowerCase() ? 'text-secondary-500' : ''}`} | ||
| data-testid='MobileNav-language-item' | ||
| > | ||
| {lang.text} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility of the language selection menu.
The current implementation has two potential accessibility issues:
- Using a div with onClick instead of a button for the language menu trigger
- Missing ARIA attributes for the expandable menu
Consider this improvement:
- <div className='space-y-2 px-5 py-2' onClick={() => showMenu('language')}>
+ <div className='space-y-2 px-5 py-2'>
+ <button
+ onClick={() => showMenu('language')}
+ aria-expanded={open === 'language'}
+ aria-controls="language-menu"
+ className="w-full text-left"
+ >
<div className='grid gap-4'>
<div>
<h4 className='mb-4 flex justify-between font-medium text-gray-800'>
<a className='flex cursor-pointer items-center gap-x-2'>
Language <IconLanguage />
</a>
<NavItemDropdown />
</h4>
- {open === 'language' &&
+ <div
+ id="language-menu"
+ role="menu"
+ className={open === 'language' ? 'block' : 'hidden'}
+ >
uniqueLangs.map((lang) => (
<button
key={lang.key}
onClick={() => changeLanguage(lang.value.toLowerCase(), true)}
+ role="menuitem"
className={`mb-4 ml-2 block w-full rounded-lg py-1 text-start text-sm font-medium leading-6 text-gray-700 transition duration-150 ease-in-out hover:bg-gray-50 ${
currentLanguage.toLowerCase() === lang.text.toLowerCase() ? 'text-secondary-500' : ''
}`}
data-testid='MobileNav-language-item'
>
{lang.text}
</button>
- ))}
+ )}
+ </div>
</div>
</div>
+ </button>
</div>Committable suggestion skipped: line range outside the PR's diff.
asyncapi-bot
requested review from
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
ADDING_TRANSLATIONS.md (4)
Line range hint
21-30: Update directory structure examples for consistencyThe directory structure examples show the correct full language names (
deutsch,english), but there's an inconsistency in the ellipsis notation. The JSON files list ends with both┗ 📜tools.jsonand┗ 📜....json.📦locales ┣ 📂deutsch ┃ ┣ 📜common.json ┃ ┣ 📜landing-page.json - ┃ ┗ 📜tools.json - ┃ ┗ 📜....json + ┃ ┣ 📜tools.json + ┃ ┗ 📜....json🧰 Tools
🪛 Markdownlint (0.37.0)
19-19: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
134-135: Improve clarity in routing configuration instructionsThe instructions for configuring i18n routing could be clearer:
- The phrase "exact same" is redundant
- The example immediately jumps to newsletter page without proper context
- - Make sure to add the exact same `href` to the `utils/i18n.ts` in the respective locales which support that `href`. + - Add the corresponding `href` to the `utils/i18n.ts` file for each supported locale. - For example, if you want to translate the `pages/newsletter.tsx` page, so that if someone visits `asyncapi.com/deutsch/newsletter`, it shows the page in the `German` locale. + For example, to translate the `pages/newsletter.tsx` page and make it accessible at `asyncapi.com/deutsch/newsletter` in German:Also applies to: 137-139
🧰 Tools
🪛 LanguageTool
[style] ~135-~135: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ng referenced. - Make sure to add the exact samehrefto theutils/i18n.tsin the re...(EN_WORDINESS_PREMIUM_EXACT_SAME)
🪛 Markdownlint (0.37.0)
134-134: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
135-135: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
244-246: Fix grammatical error in locale descriptionThere's a grammatical error in the description of existing locales.
-There exist a few locales like `english` (English) and `deutsch` (German) which have available localizations present. +There exists a few locales like `english` (English) and `deutsch` (German) which have available localizations.🧰 Tools
🪛 LanguageTool
[grammar] ~244-~244: Did you mean “exists”?
Context: ... people from all over the world. There exist a few locales likeenglish(English) ...(THERE_VBP_A_NN)
134-135: Fix Markdown formatting inconsistenciesThe document contains inconsistent list indentation levels. According to Markdownlint:
- Unordered lists should have consistent indentation (0 spaces expected, 2 spaces found)
- Some sections use emphasis (
**Note**) where headings would be more appropriateConsider using a Markdown linter to automatically fix these formatting issues.
Also applies to: 249-252, 255-256, 260-260
🧰 Tools
🪛 LanguageTool
[style] ~135-~135: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ng referenced. - Make sure to add the exact samehrefto theutils/i18n.tsin the re...(EN_WORDINESS_PREMIUM_EXACT_SAME)
🪛 Markdownlint (0.37.0)
134-134: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
135-135: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
components/navigation/NavBar.tsx (2)
46-46: Consider improving language name handling.The language name transformation has a few areas for improvement:
- The hard-coded "English" string should use i18n for consistency
- The capitalization logic is duplicated across the component
- Missing validation against supported languages list
Consider extracting the language transformation logic into a utility function:
+ const transformLanguageName = (lang: string): string => { + const i18nKey = `languages.${lang.toLowerCase()}`; + const translatedName = t(i18nKey); + return translatedName.charAt(0).toUpperCase() + translatedName.slice(1); + }; const getUniqueLangs = (): string[] => { // ... existing path handling ... const uniqueLangs = Object.keys(i18nPaths) .filter((lang) => i18nPaths[lang].includes(pathnameWithoutLocale)) - .map((lang) => lang.charAt(0).toUpperCase() + lang.slice(1)); + .filter((lang) => languages.includes(lang.toLowerCase())) + .map(transformLanguageName); - return uniqueLangs.length === 0 ? ['English'] : uniqueLangs; + return uniqueLangs.length === 0 ? [transformLanguageName(defaultLanguage)] : uniqueLangs; };Also applies to: 59-62
236-236: Consolidate language name transformation logic.The capitalization logic is duplicated here. This should use the same utility function suggested for
getUniqueLangs.- selected={i18n.language ? i18n.language.charAt(0).toUpperCase() + i18n.language.slice(1) : 'English'} + selected={transformLanguageName(i18n.language || defaultLanguage)}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ADDING_TRANSLATIONS.md(10 hunks)components/navigation/NavBar.tsx(6 hunks)
🧰 Additional context used
🪛 LanguageTool
ADDING_TRANSLATIONS.md
[style] ~135-~135: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ng referenced. - Make sure to add the exact same href to the utils/i18n.ts in the re...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
[grammar] ~244-~244: Did you mean “exists”?
Context: ... people from all over the world. There exist a few locales like english (English) ...
(THERE_VBP_A_NN)
[style] ~250-~250: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... folder with the name of the locale you want to introduce. - Create new JSON files ...
(REP_WANT_TO_VB)
[style] ~251-~251: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f the locale you want to introduce. - Create new JSON files with the same name as ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Markdownlint (0.37.0)
ADDING_TRANSLATIONS.md
134-134: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
135-135: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
139-139: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
170-170: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
249-249: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
250-250: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
251-251: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
252-252: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
255-255: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
256-256: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
260-260: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
295-295: Expected: 0; Actual: 1
Unordered list indentation
(MD007, ul-indent)
248-248: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
254-254: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (3)
ADDING_TRANSLATIONS.md (1)
Line range hint
297-312: LGTM: i18n paths configuration exampleThe i18n paths configuration example correctly demonstrates how to add routes for a new locale while maintaining consistency with existing patterns.
🧰 Tools
🪛 LanguageTool
[grammar] ~244-~244: Did you mean “exists”?
Context: ... people from all over the world. There exist a few locales likeenglish(English) ...(THERE_VBP_A_NN)
[style] ~250-~250: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... folder with the name of the locale you want to introduce. - Create newJSONfiles ...(REP_WANT_TO_VB)
[style] ~251-~251: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f the locale you want to introduce. - Create newJSONfiles with the same name as ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Markdownlint (0.37.0)
249-249: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
250-250: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
251-251: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
252-252: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
255-255: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
256-256: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
260-260: Expected: 0; Actual: 2
Unordered list indentation(MD007, ul-indent)
295-295: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
248-248: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
254-254: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
components/navigation/NavBar.tsx (2)
150-150: LGTM! Layout improvements enhance responsiveness.The spacing and alignment changes improve the UI in both mobile and desktop views.
Also applies to: 181-181
250-257: Verify mobile menu behavior during language switch.The mobile menu integration looks good, but there are two considerations:
- The currentLanguage transformation should use the suggested utility function
- Verify that the mobile menu state is properly reset after language switch
<MobileNavMenu onClickClose={() => setMobileMenuOpen(false)} uniqueLangs={uniqueLangs} - currentLanguage={i18n.language ? i18n.language.charAt(0).toUpperCase() + i18n.language.slice(1) : 'English'} + currentLanguage={transformLanguageName(i18n.language || defaultLanguage)} changeLanguage={changeLanguage} />✅ Verification successful
Mobile menu behavior during language switch is properly implemented.
The mobile menu automatically closes after language switch because:
- The menu state is reset via useEffect hook when the path changes
- Language changes trigger path updates through router navigation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if mobile menu state is properly handled during language switch # Search for mobile menu state handling in MobileNavMenu rg -A 5 "MobileNavMenu.*onClickClose" --type tsx # Check for any useEffect hooks handling language changes rg "useEffect.*i18n\.language" --type tsxLength of output: 150
Script:
#!/bin/bash # Search for mobile menu implementation and language-related effects # Using broader file type matching # Check mobile menu implementation rg -t ts -t tsx -A 5 "MobileNavMenu.*onClickClose" # Look for language change handlers rg -t ts -t tsx "useEffect.*language" # Search for mobile menu state management rg -t ts -t tsx "setMobileMenuOpen" # Check MobileNavMenu component implementation fd -e ts -e tsx -x grep -l "MobileNavMenu" {} \; | xargs catLength of output: 17009
| - Navigate to the `next-i18next.config.js` file in the root of the project folder. | ||
| - Add the name of the newly added `locale` to the `languages` array. | ||
|
|
||
| **3. Configure i18n routing** | ||
| After adding a new internationalized page, ensure it is being served on the website when someone visits. | ||
| - Make sure to add the same `href` to the `lib/i18nPaths.js` in the respective locales supporting that `href`. | ||
| - Make sure to add the same `href` to the `utils/i18n.ts` in the respective locales supporting that `href`. | ||
|
|
||
| If you have added the 'fr' locale and translated the 'tools/cli' page, clicking 'Tools -> CLI' in the navigation menu will redirect the user to 'asyncapi.com/fr/tools/cli'. | ||
| If you have added the 'french' locale and translated the 'tools/cli' page, clicking 'Tools -> CLI' in the navigation menu will redirect the user to 'asyncapi.com/french/tools/cli'. | ||
|
|
||
| `locales` folder structure | ||
| ```diff | ||
| locales | ||
| ┣ de | ||
| ┣ deutsch | ||
| ┃ ┣ common.json | ||
| ┃ ┣ landing-page.json | ||
| ┃ ┗ tools.json | ||
| ┣ en | ||
| ┣ english | ||
| ┃ ┣ common.json | ||
| ┃ ┣ landing-page.json | ||
| ┃ ┗ tools.json | ||
| + ┗ fr | ||
| + ┗ french | ||
| + ┃ ┣ common.json | ||
| + ┃ ┣ landing-page.json | ||
| + ┃ ┗ tools.json | ||
| ``` | ||
|
|
||
| - Change the `next-i18next-static-site.config.js` config. | ||
| - Change the `next-i18next.config.js` config. | ||
|
|
||
| `next-i18next-static-site.config.js` | ||
| `next-i18next.config.js` | ||
| ```diff | ||
| module.exports = { | ||
| i18n: { | ||
| - languages: ["en", "de"], | ||
| + languages: ["en", "de", "fr"], | ||
| defaultLanguage: "en", | ||
| - languages: ["english", "deutsch"], | ||
| + languages: ["english", "deutsch", "french"], | ||
| defaultLanguage: "english", |
There was a problem hiding this comment.
💡 Codebase verification
Documentation inconsistency confirmed
The documentation refers to two different configuration files:
next-i18next-static-site.config.js(incorrect reference)next-i18next.config.js(actual file)
The correct configuration file name is next-i18next.config.js, as verified in the repository. The reference to next-i18next-static-site.config.js should be updated to maintain consistency.
🔗 Analysis chain
Verify configuration file references
The documentation refers to next-i18next.config.js, but earlier sections mention next-i18next-static-site.config.js. This inconsistency needs to be addressed.
Let's verify the correct configuration file name:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check which configuration file exists in the repository
fd -e js -E 'node_modules' 'next-i18next.*config'
Length of output: 72
Script:
#!/bin/bash
# Search for any mentions of the static site config file
rg "next-i18next-static-site\.config"
Length of output: 159
🧰 Tools
🪛 Markdownlint (0.37.0)
255-255: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
256-256: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
260-260: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
|
@asyncapi/bounty_team |
public/locales/deutsch/common.json
Outdated
There was a problem hiding this comment.
IMHO, we should keep it as de and en. I haven't encountered a website using full names in URLs.
The dropdown should have full names like Deutsch but the URLs should still point to /de or /en and not the full /deutsch
There was a problem hiding this comment.
@sambhavgupta0705 suggested this change in the #3113. But I can revert this change.
There was a problem hiding this comment.
Please revert the changes related to en -> english here and in i18n config file since URLs should be having smaller prefix
There was a problem hiding this comment.
Okay I will do that!
|
The design-related changes looks good |
Co-authored-by: Ansh Goyal <[email protected]>
|
@anshgoyalevil Do I need to fix the Lighthouse Failing errors in this PR due to the image and CSS part: |
|
Yes, If the regression is caused by this PR (but I don't think it is). Otherwise, you can ignore it and solve (if willing) it in another PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/icons/Language.tsx (2)
4-6: Enhance JSDoc documentation with props description.Add
@paramand@returnstags to document the component's interface./** * @description Language Icon for language selector component + * @param {Object} props - Component props + * @param {string} [props.className=''] - Additional CSS classes to apply + * @returns {JSX.Element} Language icon SVG */
7-7: Add TypeScript type definitions.Since this is a TypeScript file, add proper type definitions for the component props.
-export default function IconLanguage({ className = '' }) { +interface IconLanguageProps { + className?: string; +} + +export default function IconLanguage({ className = '' }: IconLanguageProps) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/icons/Language.tsx(1 hunks)
🔇 Additional comments (2)
components/icons/Language.tsx (2)
3-3: Consider removing eslint-disable comment.The
max-lendisable comment might be unnecessary if the SVG path can be formatted across multiple lines.Let's check if other icon components use similar eslint-disable comments:
9-23: SVG implementation looks good!The SVG implementation follows best practices:
- Uses currentColor for dynamic color inheritance
- Allows className customization
- Maintains consistent stroke width
- Uses appropriate viewBox dimensions
Note: The
size-5class suggests Tailwind CSS usage. Ensure this utility class is available in your Tailwind configuration.Let's verify the Tailwind configuration:
✅ Verification successful
Tailwind size utility verification successful
The
size-5utility class is valid and follows the project's established patterns. Tailwind CSS is properly configured in the project, and similar size utilities are consistently used across 40+ components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Tailwind configuration for size utilities rg -l "size-" "tailwind.config.*"Length of output: 91
Script:
#!/bin/bash # Check for Tailwind dependencies and configuration echo "=== Package.json check ===" fd -t f "package.json" --exec cat {} \; | grep -i "tailwind" echo -e "\n=== Searching for Tailwind config files ===" fd -t f "(tailwind|postcss)\.config\." --hidden echo -e "\n=== Checking size utility usage ===" rg "size-[0-9]" -lLength of output: 1996
Description
This PR enables the i18n feature for smaller screens. Additionally, the following changes have been made:
Related issue(s)
Resolves #2708
Summary by CodeRabbit
Release Notes
New Features
Improvements
Changes