-
Notifications
You must be signed in to change notification settings - Fork 100
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
fix: DateSelector selector validation message issues #823
fix: DateSelector selector validation message issues #823
Conversation
@@ -20,7 +21,7 @@ export function DateSelector({ time, onChange, customLabel, minDate }: DataAndTi | |||
return '' | |||
} | |||
} | |||
}, [error]) | |||
}, [error, t]) |
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.
Maybe it would be better if we'll just remove the useMemo
hook
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.
@NoamGaash
It's a good idea to remove it.
while removing it I had thought about 2 how to implement it.
- Just removing the useMemo and use the function.
- Take the function out of the component( and remove the useTranslation also) - will look better.
does it matter which approach I go forward with? I did both of them and it works.
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.
That's up to you
I'd like to make a third suggestion -
- the function could return the translation key
- the function can be inside the scope of the component, or underneath
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.
I made a suggestion - it will let you keep using useTranslation
which is better approach as it will trigger re-render on language change
@NoamGaash |
@YuvalMasada I'll try |
@NoamGaash - This time at the right direction
I've added 2 short fixes.