-
Notifications
You must be signed in to change notification settings - Fork 26
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: Adapt search placeholder if assistant is disabled #2255
Conversation
BundleMonFiles updated (1)
Unchanged files (12)
Total files change +11B 0% Final result: ✅ View report in BundleMon website ➡️ |
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.
ça utilise la SearchBar au final, qui a le wording souhaité par défaut 🤔 est-ce que t'as essayé en supprimant le placeholder plutôt qu'en le remplaçant ? Histoire de retomber sur le cas par défaut (sinon faut regarder la doc pour savoir comment retomber dans le cas par défaut)
a183040
to
c0120ab
Compare
ah je ne savais pas qu'on avait un fallback automagique, j'ai amend le commit |
@@ -43,7 +45,9 @@ export const AssistantWrapperMobile = () => { | |||
<Icon className="u-ml-1 u-mr-half" icon={AssistantIcon} size={24} /> | |||
} | |||
type="button" | |||
label={t('assistant.search.placeholder')} | |||
label={ | |||
isAssistantEnabled ? t('assistant.search.placeholder') : '' // Fallback on SearchBar default |
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.
perso je mettrai plutôt un undefined
qu'une string vide, c'est plus clair à mon sens qu'en au fait qu'on part alors sur le comportement par défaut du composant (et donc pas besoin du commentaire)
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.
Pour moi ça ne change pas grand chose, il y a toujours besoin du commentaire pour savoir qu'il y a un fallback, mais ok pour un explicite undefined
When the assistant is enabled, the search wording suggests the user to ask a question, that could be answered by the assistant or the search. But if the assistant is not enabled, we should adapt the wording to focus on search only.
c0120ab
to
f84d0ab
Compare
Closing because moved here cozy/cozy-libs@57a1507 |
When the assistant is enabled, the search wording suggests the user to ask a question, that could be answered by the assistant or the search. But if the assistant is not enabled, we should adapt the wording to focus on search only.