Skip to content

ASAB Shell - Optimize handling/rendering the language flags #60

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.

Sign up for GitHub

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

Closed
JanMajer86 wants to merge 1 commit into from
Closed

ASAB Shell - Optimize handling/rendering the language flags #60

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/modules/i18n/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export default function LanguageDropdown(props) {
});
}

const FLAG_STYLE = {width: '32px'};

Comment on lines +25 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Move FLAG_STYLE outside the component for true memoization benefit.

Defining FLAG_STYLE inside the component function causes it to be recreated on every render, producing a new object reference each time. This defeats the optimization goal and breaks React.memo's shallow comparison on the LanguageFlag component.

Proposed fix

Move the constant outside the component:

+const FLAG_STYLE = { width: '32px' };
+
 export default function LanguageDropdown(props) {
 
 	const { t, i18n } = useTranslation();
 
 	const changeLanguage = (language) => {
 		i18n.changeLanguage(language).then(() => {
 			// if the hard refresh is needed: window.location.reload(false);
 		});
 	}
 
-	const FLAG_STYLE = {width: '32px'};
-
 	const { dispatch } = useAppStore();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/modules/i18n/dropdown.js` around lines 25 - 26, The constant FLAG_STYLE
is currently defined inside the LanguageFlag component causing a new object
reference on each render; move the FLAG_STYLE = {width: '32px'} declaration out
of the LanguageFlag function to module/top-level scope so it is created once and
reused, then remove the in-component definition and ensure LanguageFlag
references the top-level FLAG_STYLE for memoization to work with React.memo.

const { dispatch } = useAppStore();

useEffect(() => {
Expand All @@ -45,7 +47,7 @@ export default function LanguageDropdown(props) {
if (language == 'cimode') return null;
return (
<DropdownItem key={language} title={language} onClick={() => {changeLanguage(language)}}>
<LanguageFlag language={language} className="pe-2" style={{width: '32px'}}/>
<LanguageFlag language={language} className="pe-2" style={FLAG_STYLE}/>
{t('i18n|language|'+language)}
</DropdownItem>
);
Expand All @@ -56,6 +58,6 @@ export default function LanguageDropdown(props) {
);
}

function LanguageFlag({language, className, style}) {
const LanguageFlag = React.memo(function LanguageFlag({language, className, style}) {
return (<img className={className} src={`media/locale/${language}.svg`} style={style}/>)
}
});
Comment on lines +61 to +63
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add alt attribute to the <img> element for accessibility.

The flag image is missing an alt attribute. For accessibility compliance, provide descriptive alt text or use alt="" if purely decorative.

Proposed fix 
-const LanguageFlag = React.memo(function LanguageFlag({language, className, style}) {
-	return (<img className={className} src={`media/locale/${language}.svg`} style={style}/>)
-});
+const LanguageFlag = React.memo(function LanguageFlag({language, className, style}) {
+	return (<img className={className} src={`media/locale/${language}.svg`} style={style} alt={`${language} flag`}/>)
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const LanguageFlag = React.memo(function LanguageFlag({language, className, style}) {
return (<img className={className} src={`media/locale/${language}.svg`} style={style}/>)
}
});
const LanguageFlag = React.memo(function LanguageFlag({language, className, style}) {
return (<img className={className} src={`media/locale/${language}.svg`} style={style} alt={`${language} flag`}/>)
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/modules/i18n/dropdown.js` around lines 61 - 63, Update the LanguageFlag
component so the rendered <img> includes an alt attribute for accessibility:
modify the LanguageFlag function (LanguageFlag) to accept an optional alt prop
(e.g., add alt to the destructured props) or otherwise compute a descriptive
string (for example `${language} flag`) and pass it as the img alt; if the image
is purely decorative, set alt="" instead. Ensure the alt attribute is set on the
<img> element generated in LanguageFlag.

Loading