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.

Sign up for GitHub

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

Jump to bottom

Tooltip modifications #2056

Merged
merged 2 commits into from
Mar 25, 2021
Merged

Tooltip modifications #2056

merged 2 commits into from
Mar 25, 2021

Conversation

parasharrajat
Copy link
Member

Please review @roryabraham

Details

  • Added Tooltip opening and closing delay configuration and Set Opening delay to 500ms.

Fixed Issues

Fixes #1490

Tests

Try it on the web.

Tested On

  • Web

Screenshots

Web

screen.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner March 24, 2021 21:36
@botify botify requested review from deetergp and removed request for a team March 24, 2021 21:36
@parasharrajat
Copy link
Member Author

For the Animation changes mentioned here
@marcaaron @roryabraham You have to come to an agreement first. Lol. 😃

roryabraham
roryabraham previously approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@roryabraham roryabraham left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice, I think this is a big improvement. Code is nice and simple too 👍

marcaaron
marcaaron requested changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

I think the scaling effect needs to go, but I can send my own PR to remove it if we don't want to acknowledge it here. 😆

src/components/Tooltip/index.js Outdated Show resolved Hide resolved
@parasharrajat
Copy link
Member Author

parasharrajat commented Mar 25, 2021 •
edited
Loading

Updated @marcaaron. Thanks for the hint. How could I miss that?

marcaaron
marcaaron requested changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

Looks good just one small request

src/components/Tooltip/TooltipPropTypes.js Outdated Show resolved Hide resolved
@marcaaron
Copy link
Contributor

@parasharrajat please do not force push after a review has begun

@parasharrajat
Copy link
Member Author

Sure @marcaaron next time. Thanks for letting me know.

@marcaaron marcaaron self-requested a review March 25, 2021 00:36
marcaaron
marcaaron previously approved these changes Mar 25, 2021
View reviewed changes
roryabraham
roryabraham approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Nice, even simpler!

deetergp
deetergp approved these changes Mar 25, 2021
View reviewed changes
Copy link
Contributor

@deetergp deetergp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@marcaaron marcaaron merged commit 2dc397d into Expensify:master Mar 25, 2021
@OSBotify OSBotify mentioned this pull request Mar 25, 2021
Closed
85 tasks
@huzaifa-99 huzaifa-99 mentioned this pull request May 31, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deetergp deetergp deetergp approved these changes

@roryabraham roryabraham roryabraham approved these changes

@marcaaron marcaaron marcaaron left review comments

Assignees

@parasharrajat parasharrajat

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expensify chat - web/desktop, when hovering over a user's name anywhere, show their primary login details
4 participants
@parasharrajat @marcaaron @deetergp @roryabraham