-
Notifications
You must be signed in to change notification settings - Fork 4
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
Move search button in top-bar next to tabs #1778
Conversation
Also move code from commons-ui to GridStudy
a5730b2
to
b604295
Compare
src/components/app-top-bar.js
Outdated
<Search /> | ||
</IconButton> | ||
</Tooltip> | ||
</Box> |
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.
Is there any difference between the two search buttons ?
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.
oups, I forget to remove it!
Yes they don't have the same apparence (mainly one is sqaure retangle and the other is round.
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.
code: ok
tests: works fine
just a question about the added tooltip
)} | ||
{studyUuid && ( | ||
<Box sx={styles.searchButton}> | ||
<Tooltip |
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.
This tooltip was not here before.
Maybe you should ask a PO to confirm ?
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 confirmed with a PO, it's OK.
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.
Code OK
Test OK
Also move code from commons-ui to GridStudy
Don't need gridsuite/commons-ui#325 to be tested.