-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat(project): update edit and create forms #339
feat(project): update edit and create forms #339
Conversation
Visit the preview URL for this PR (updated for commit 3fae481): https://ottwebapp--pr339-feat-profiles-edit-c-fn2ntxm0.web.app (expires Thu, 17 Aug 2023 11:01:50 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c198f8a3a199ba8747819f7f1e45cf602b777529 |
{t('profile.greeting')} | ||
{`${fullName && ','} ${fullName}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to include the name placeholder in the translations because we can't guarantee that the sentence structure is the same for all languages.
{t('profile.greeting')} | |
{`${fullName && ','} ${fullName}`} | |
{fullName ? t('profile.greeting_with_name', { fullName }) : t('profile.greeting')} |
translations:
{
"profile": {
"greeting_with_name": "Greetings, {{fullName}}!"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@naumovski-filip Just a logistical question, but why are we opening PR's into feature branches? Is this code that needs to be reviewed or should it wait until the entire feature is ready? Or better yet can it be added incrementally into the develop branch? |
It should be reviewed yes, the idea is to merge |
I'm not sure exactly how that is going to work though. The code in the hackweek branch and the |
Description
Config: maqet3qh
https://www.figma.com/file/qLl8KxBOOr640h18ICx0H6/Web-app?type=design&node-id=2344-62912&mode=design
Steps completed:
According to our definition of done, I have completed the following steps: