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.

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

Hide tooltip when hovering over the attribution group and between top level groups. #413

Merged
merged 2 commits into from Jan 20, 2021

Conversation

stanislawosinski
Copy link
Contributor

@stanislawosinski stanislawosinski commented Jan 19, 2021

No description provided.

@valscion
Copy link
Member

@valscion valscion commented Jan 20, 2021

Thanks! Would you be able to showcase the difference in behavior with a small screencap or something? It would make it easier to understand how this behavior has changed now ☺️

@stanislawosinski
Copy link
Contributor Author

@stanislawosinski stanislawosinski commented Jan 20, 2021

Before:

tooltips-before.mp4

After:

tooltip-after.mp4

@valscion
Copy link
Member

@valscion valscion commented Jan 20, 2021

Nice, thank you! Looks great!

Would you be able to write a small note on the changelog about this?

event.preventDefault();
if (props.onMouseLeave) {
Copy link
Member

@valscion valscion Jan 20, 2021

Choose a reason for hiding this comment

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

Do we need this conditional? Is there a place in the code where props.onMouseLeave would not be specified?

Copy link
Contributor Author

@stanislawosinski stanislawosinski Jan 20, 2021

Choose a reason for hiding this comment

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

I made the condition analogous to the props.onGroupHover check below, which is also there but probably not required:

        if (props.onGroupHover) {
          props.onGroupHover.call(component, event);
        }

I can remove the props.onMouseLeave condition, not a problem.

th0r
th0r approved these changes Jan 20, 2021
@th0r th0r merged commit 9ade716 into webpack-contrib:master Jan 20, 2021
5 checks passed
@th0r
Copy link
Collaborator

@th0r th0r commented Jan 20, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants