feat: added autocomplete for language codes #117
Conversation
There was a problem hiding this comment.
@shubham-1806 Thank you for the good work! There are some changes that I have requested, which can be easily rectified. The autocomplete component's usage is very good and clean!
taxonomy-editor-frontend/src/pages/editentry/ListTranslations.jsx
Outdated
Show resolved
Hide resolved
|
@aadarsh-ram Please review the changes |
There was a problem hiding this comment.
LGTM @shubham-1806! I'd like @nobeeakon to review this as well.
| @@ -17,3 +17,189 @@ function taxonomyApiUrlFromUi(location) { | |||
| } | |||
| export const API_URL = taxonomyApiUrlFromUi(window.location); | |||
| export const ENTER_KEYCODE = 13; | |||
| export const ISO_CODES ={ | |||
There was a problem hiding this comment.
could we get this from the library? I think it is going to be difficult to maintain this
| options={Object.keys(ISO_CODES)} | ||
| onChange={(e,language) => { | ||
| setNewLanguageCode(ISO_CODES[language]); | ||
| setIsValidLanguageCode(ISO6391.validate(ISO_CODES[language])) |
There was a problem hiding this comment.
guess this could be a derived state:
const isValidLanguageCode = ISO6391.validate(newLanguageCode)
instead of a useState
| onChange={(e,language) => { | ||
| setNewLanguageCode(ISO_CODES[language]); | ||
| const validateBool = ISO6391.validate(ISO_CODES[language]) | ||
| const ifDuplicateBool = renderedTranslations.some(el => (el.languageCode === ISO_CODES[language])) || |
There was a problem hiding this comment.
nit: let's rename these two: isValidLanguage and isDuplicateLanguage?
There was a problem hiding this comment.
guess this could be a derived state:
const isValidLanguageCode = ISO6391.validate(newLanguageCode)
...
instead of a useState
| options={Object.keys(ISO_CODES)} | ||
| onChange={(e,language) => { | ||
| setNewLanguageCode(ISO_CODES[language]); | ||
| setIsValidLanguageCode(ISO6391.validate(ISO_CODES[language])) |
There was a problem hiding this comment.
guess this could be a derived state:
const isValidLanguageCode = ISO6391.validate(newLanguageCode)
instead of a useState
| setNewLanguageCode(e.target.value); | ||
| setIsValidLanguageCode(ISO6391.validate(e.target.value)); | ||
| <Autocomplete | ||
| options={Object.keys(ISO_CODES)} |
There was a problem hiding this comment.
think we don't need to have the ISO_CODES const. Please check the iso-639-1 package docs, we can obtain the names and extract the codes with it.
| }} | ||
| label="Language Code" | ||
| error={!isValidLanguageCode} | ||
| renderInput={(params) => <TextField error={!isValidLanguageCode} {...params} label="Languages" />} |
There was a problem hiding this comment.
nit: does it needs a helper text as the other one?
There was a problem hiding this comment.
thanks for the PR , nice work, just please replace the ISO_CODES , think we can use the features that the iso-639-1 package already gives us
nobeeakon
changed the title
nobeeakon
changed the title
What
Added Autocomplete for selecting language code wherever necessary using autocomplete component of MUI.
Screenshot
Fixes bug(s)
Fixes #107