Fix filtering when searching for languages in app-hosted mode (BL-15822)#604
Conversation
dd4c6dd to
e92c405
Compare
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson reviewed 1 file and made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @andrew-polk).
src/components/appHosted/AppHostedLanguageGroup.tsx line 114 at r2 (raw file):
: matchingLanguages.filter( (lang) => !preferredLangCodes.includes(lang.isoCode) );
Not sure how picky to be here. I find that using negations in conditionals that have both branches is usually less clear than using the positive and reversing the then and else. In this case, I think it would be even clearer to have
languagesToDisplay = matchingLanguages.
if (showPreferredLangs) {
// don't show the preferred languages in both places; screen space is precious...
...
But what you have is reasonably clear and will work.
e92c405 to
518cd3a
Compare
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @JohnThomson).
src/components/appHosted/AppHostedLanguageGroup.tsx line 114 at r2 (raw file):
Previously, JohnThomson (John Thomson) wrote…
Not sure how picky to be here. I find that using negations in conditionals that have both branches is usually less clear than using the positive and reversing the then and else. In this case, I think it would be even clearer to have
languagesToDisplay = matchingLanguages.
if (showPreferredLangs) {
// don't show the preferred languages in both places; screen space is precious...
...
But what you have is reasonably clear and will work.
I agree that is much better.
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @andrew-polk).
This change is