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

Cleanup tooltip & popover components #34205

Merged
merged 6 commits into from Jul 22, 2021
Merged

Cleanup tooltip & popover components #34205

merged 6 commits into from Jul 22, 2021

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented Jun 8, 2021 •
edited

Do some cleanup on tooltip and popover

  • refactor duplicated code to functions
  • use newest methods

NOTE for reviewers:

Better review it, commit by commit. It will help you with the proper message and will guide you with sanity to follow the logic

preview

@GeoSot GeoSot marked this pull request as ready for review June 8, 2021 00:17
@GeoSot GeoSot requested a review from a team as a code owner June 8, 2021 00:17
@GeoSot GeoSot added this to Inbox in v5.1.0 via automation Jun 8, 2021
@GeoSot GeoSot moved this from Inbox to Review in v5.1.0 Jun 11, 2021
@GeoSot GeoSot force-pushed the gs-tooltip-cleanup branch 3 times, most recently from 7df354c to 1bee4a9 Compare June 22, 2021 20:05
rohit2sharma95
rohit2sharma95 reviewed Jun 23, 2021
View reviewed changes
js/src/popover.js Outdated Show resolved Hide resolved
js/src/tooltip.js Show resolved Hide resolved
js/src/tooltip.js Outdated Show resolved Hide resolved
js/src/tooltip.js Outdated Show resolved Hide resolved
v5.1.0 automation moved this from Review to Approved Jul 5, 2021
rohit2sharma95
rohit2sharma95 approved these changes Jul 5, 2021
View reviewed changes
Copy link
Collaborator

@rohit2sharma95 rohit2sharma95 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Better to have one more review from @twbs/js-review as I am very low at regex 😁

js/src/tooltip.js Outdated Show resolved Hide resolved
@GeoSot GeoSot force-pushed the gs-tooltip-cleanup branch 2 times, most recently from b191c69 to 7192509 Compare July 9, 2021 13:02
@GeoSot GeoSot changed the title Some cleanup on tooltip && popover Some cleanup on tooltip & popover Jul 9, 2021
@GeoSot GeoSot requested a review from ffoodd July 12, 2021 14:43
@GeoSot GeoSot mentioned this pull request Jul 16, 2021
Merged
3 tasks
XhmikosR
XhmikosR reviewed Jul 20, 2021
View reviewed changes
js/src/tooltip.js Outdated Show resolved Hide resolved
@GeoSot GeoSot force-pushed the gs-tooltip-cleanup branch 2 times, most recently from 63f15f8 to 9dc1cb3 Compare July 20, 2021 15:58
@XhmikosR XhmikosR force-pushed the gs-tooltip-cleanup branch 3 times, most recently from ae54c4f to 865f918 Compare July 22, 2021 13:51
@XhmikosR XhmikosR changed the title Some cleanup on tooltip & popover Cleanup tooltip & popover components Jul 22, 2021
@XhmikosR XhmikosR merged commit da2db21 into main Jul 22, 2021
v5.1.0 automation moved this from Approved to Done Jul 22, 2021
@XhmikosR XhmikosR deleted the gs-tooltip-cleanup branch July 22, 2021 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@folknor folknor folknor left review comments

@XhmikosR XhmikosR XhmikosR approved these changes

@rohit2sharma95 rohit2sharma95 rohit2sharma95 approved these changes

@alpadev alpadev Awaiting requested review from alpadev

@ffoodd ffoodd Awaiting requested review from ffoodd

Assignees
No one assigned
Labels
js no squash refactoring v5
Projects
No open projects
v5.1.0
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@GeoSot @folknor @XhmikosR @rohit2sharma95