Skip to content
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.

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

LanguageFood on develop branch #349

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

pl0ss
Copy link
Contributor

@pl0ss pl0ss commented Oct 26, 2023

🤖 Generated by Copilot at 91e571b

Summary

🌐🍽️🛠️

This pull request adds a new feature to the food section of the app, which allows the user to select a language for the food menu (default, German or English). It modifies the FoodCard, FilterFoodModal, useFoodFilter and Food components, as well as the common.json files for the German and English locales, to implement the feature.

Oh we are the coders of the FoodCard crew
And we like to see the menus in different languages too
So we added a feature to the FilterFoodModal view
And we heave away and haul away and pull the code review

Walkthrough

  • Add a new feature to allow the user to select the language for the food menu (default, de or en) (link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Use the selectedLanguageFood prop and state to access and update the user's choice of language for the food menu in the FoodCard, FilterFoodModal and Food components (link, link, link, link, link, link, link, link, link, link)
  • Replace the use of i18n.languages[0] with languageFood variable in the FoodCard and Food components to display the food names and descriptions in the user's chosen language for the food menu (link, link, link, link, link, link, link, link)

@Robert27 Robert27 requested a review from BuildmodeOne October 26, 2023 11:57
"title":"Sprache für den Speiseplan",
"german":"Deutsch",
"english":"Englisch",
"default":"Standart"
Copy link
Member

Choose a reason for hiding this comment

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

Should be "Standard" :)

@@ -17,6 +17,12 @@
"showReimannsStatic": "Reimanns anzeigen (feste Gerichte)",
"showCanisius": "Canisiuskonvikt anzeigen"
},
"languageFood":{
"title":"Sprache für den Speiseplan",
"german":"Deutsch",
Copy link
Member

@BuildmodeOne BuildmodeOne Oct 26, 2023

Choose a reason for hiding this comment

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

We already have the translated Lang names in the languages.json. Therefore, we shouldn't redefine them here, especially when we add new languages.

@@ -17,6 +17,12 @@
"showReimannsStatic": "Show Reimanns (static meals)",
"showCanisius": "Show Canisius Convict"
},
"languageFood":{
"title":"Language for food menu",
"german":"German",
Copy link
Member

Choose a reason for hiding this comment

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

Same here

onChange={() => toggleSelectedLanguageFood('default')}
type={'radio'} // ohne type ist es eine checkbox
/>
<Form.Check
Copy link
Member

Choose a reason for hiding this comment

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

In the best case, these options should be created dynamically based on the supported languages (see LanguageModal.jsx).

preferencesSelection,
allergenSelection
} = useContext(FoodFilterContext)
const { i18n, t } = useTranslation(['dashboard', 'food'])

let languageFood = ''
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified to:

  const languageFood = selectedLanguageFood && selectedLanguageFood !== 'default' ? selectedLanguageFood : i18n.languages[0]

Copy link
Member

Choose a reason for hiding this comment

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

I do not think the && and ? : operators should be mixed without braces, as the precendence might not be clear to the reader

@@ -30,6 +38,7 @@ import { useEffect, useState } from 'react'
* @returns {void}
*/
export function useFoodFilter () {
const [selectedLanguageFood, setSelectedLanguageFood] = useState([])
Copy link
Member

Choose a reason for hiding this comment

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

To use the proposed simplification, change the useState default to 'default'

@@ -59,7 +60,15 @@ export default function Mensa () {
const userKind = useUserKind()
const router = useRouter()
const { i18n, t } = useTranslation('food')
const currentLocale = i18n.languages[0]

let languageFood = ''
Copy link
Member

Choose a reason for hiding this comment

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

Can be simplified to:

const currentLocale = selectedLanguageFood && selectedLanguageFood !== 'default' ? selectedLanguageFood : i18n.languages[0]

Therefore, all the languageFood referenced should be changed to currentLocale

@BuildmodeOne
Copy link
Member

@pl0ss We can also look at the PR tomorrow again in detail at the Hackathon.

Thanks a lot for your improvements :)

@BuildmodeOne BuildmodeOne added the hackathon Will be solved during the next neuland.app Hackathon label Oct 26, 2023
@BuildmodeOne BuildmodeOne merged commit 309aaa0 into neuland-ingolstadt:develop Oct 27, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hackathon Will be solved during the next neuland.app Hackathon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants