Skip to content

Conversation

@mledl
Copy link
Contributor

@mledl mledl commented Aug 30, 2024

This PR adds the basic languages page including the following features:

  • LanguageSelect with available languages
  • LanguageTable with add, delete & upsert functionality including corresponding CRUD
  • Corresponding Routing

What is missing and will be done in a subsequent PR:

  • E2E Testing
  • Fallback language input field as select with available options
  • Confirmation Modal before deleting a language

mledl added 28 commits July 13, 2024 17:31
@mledl mledl requested a review from sjaghori as a code owner August 30, 2024 16:10
Copy link
Collaborator

@sjaghori sjaghori left a comment

Choose a reason for hiding this comment

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

Thank you for the PR 🚀
There are some minor issues with the validation, woud you like to have a look at that?

@mledl mledl requested a review from sjaghori September 24, 2024 20:42
sjaghori
sjaghori previously approved these changes Sep 28, 2024
Copy link
Collaborator

@sjaghori sjaghori left a comment

Choose a reason for hiding this comment

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

Great job! 🚀 I think we can merge this already.
There are just three things we should discuss:

  1. IMO it makes more sense to automatically select the fallback language when a user adds a new language. It is perfectly fine to give them the option to deselect the fallback language or choose another fallback language.
  2. We should consider renaming base language to default language
  3. Adding a modal when there are unsaved changes or removing the save option, adding the functionality for what you see is what is in the db.

@sjaghori
Copy link
Collaborator

sjaghori commented Sep 28, 2024

Oh and please add a little margin of 1rem (mt-4) between the selector and the table:

Screenshot from 2024-09-28 12-29-10

@mledl
Copy link
Contributor Author

mledl commented Sep 29, 2024

Great job! 🚀 I think we can merge this already. There are just three things we should discuss:

  1. IMO it makes more sense to automatically select the fallback language when a user adds a new language. It is perfectly fine to give them the option to deselect the fallback language or choose another fallback language.
  2. We should consider renaming base language to default language
  3. Adding a modal when there are unsaved changes or removing the save option, adding the functionality for what you see is what is in the db.

Will create a follow up PR for your third point. Will introduce both, improved save button state as well as the modal if there are stale changes. Moreover, I will set a the base language as fallback after creating a new language.

@sjaghori sjaghori merged commit 87656fd into main Sep 29, 2024
@sjaghori sjaghori deleted the feat/languages-page branch September 29, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants