-
-
Notifications
You must be signed in to change notification settings - Fork 527
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: support for disabled tooltip callback #1211
feat: support for disabled tooltip callback #1211
Conversation
WalkthroughThe recent changes introduce a new optional prop, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/components/Tooltip/Tooltip.tsx (1)
463-465
: Ensure consistent code style and improve readability.The conditional check for
disableTooltip
can be made more readable by using early return.- if(disableTooltip?.(anchor)){ - return; - } + if (disableTooltip?.(anchor)) { + return + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/components/Tooltip/Tooltip.tsx (2 hunks)
- src/components/Tooltip/TooltipTypes.d.ts (1 hunks)
- src/components/TooltipController/TooltipController.tsx (2 hunks)
- src/components/TooltipController/TooltipControllerTypes.d.ts (1 hunks)
Additional comments not posted (4)
src/components/TooltipController/TooltipControllerTypes.d.ts (1)
97-97
: New methoddisableTooltip
added toITooltipController
.The method is correctly typed and aligns with the described functionality.
src/components/Tooltip/TooltipTypes.d.ts (1)
155-155
: New methoddisableTooltip
added toITooltip
.The method is correctly typed and aligns with the described functionality.
src/components/TooltipController/TooltipController.tsx (2)
64-64
: New parameterdisableTooltip
added toTooltipController
props.The parameter is correctly added to the destructuring assignment in the component definition.
374-374
: New parameterdisableTooltip
added toTooltip
props.The parameter is correctly added to the props passed to the
Tooltip
component.
@gabrieljablonski do let me know if we need to update /examples for this. Also if everything looks fine at first glance will start adding changes to /docs. |
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.
Looks good for a first pass. If you don't mind, please add it to the "options" table on the docs.
Also, if you could try adding a test for the new prop under this tests file, we'd appreciate it. No need to waste too much time on it though if you can't figure it out.
When those are done we'll do a last check again and see if we catch anything. The feature is simple enough, but just to make sure.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
src/test/__snapshots__/tooltip-props.spec.js.snap
is excluded by!**/*.snap
Files selected for processing (3)
- docs/docs/options.mdx (1 hunks)
- src/components/Tooltip/Tooltip.tsx (2 hunks)
- src/test/tooltip-props.spec.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/components/Tooltip/Tooltip.tsx
Additional comments not posted (3)
src/test/tooltip-props.spec.js (2)
185-197
: LGTM! The test case correctly validates thedisableTooltip
prop when it returnstrue
.The test ensures that the tooltip is not rendered when
disableTooltip
returnstrue
.
199-214
: LGTM! The test case correctly validates thedisableTooltip
prop when it returnsfalse
.The test ensures that the tooltip is rendered when
disableTooltip
returnsfalse
.docs/docs/options.mdx (1)
130-130
: LGTM! The documentation clearly explains thedisableTooltip
property.The documentation correctly describes the new
disableTooltip
property and its usage.
|
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.
LGTM. Possibly merging sometime next week.
## [1.0.2](https://github.com/equinor/webviz-subsurface-components/compare/wsc-common@1.0.1...wsc-common@1.0.2) (2024-10-08) ### Bug Fixes * bump react-tooltip from 5.27.0 to 5.28.0 in /typescript ([#2300](#2300)) ([e355c99](e355c99)), closes [#1209](#1209) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1203](ReactTooltip/react-tooltip#1203) [ReactTooltip/react-tooltip#1205](ReactTooltip/react-tooltip#1205) [#1209](#1209) [#1213](#1213)
## [1.5.6](https://github.com/equinor/webviz-subsurface-components/compare/well-completions-plot@1.5.5...well-completions-plot@1.5.6) (2024-10-08) ### Bug Fixes * bump react-tooltip from 5.27.0 to 5.28.0 in /typescript ([#2300](#2300)) ([e355c99](e355c99)), closes [#1209](#1209) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1203](ReactTooltip/react-tooltip#1203) [ReactTooltip/react-tooltip#1205](ReactTooltip/react-tooltip#1205) [#1209](#1209) [#1213](#1213)
## [1.0.3](https://github.com/equinor/webviz-subsurface-components/compare/subsurface-viewer@1.0.2...subsurface-viewer@1.0.3) (2024-10-08) ### Bug Fixes * bump react-tooltip from 5.27.0 to 5.28.0 in /typescript ([#2300](#2300)) ([e355c99](e355c99)), closes [#1209](#1209) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1203](ReactTooltip/react-tooltip#1203) [ReactTooltip/react-tooltip#1205](ReactTooltip/react-tooltip#1205) [#1209](#1209) [#1213](#1213)
## [1.3.11](https://github.com/equinor/webviz-subsurface-components/compare/group-tree-plot@1.3.10...group-tree-plot@1.3.11) (2024-10-08) ### Bug Fixes * bump react-tooltip from 5.27.0 to 5.28.0 in /typescript ([#2300](#2300)) ([e355c99](e355c99)), closes [#1209](#1209) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1203](ReactTooltip/react-tooltip#1203) [ReactTooltip/react-tooltip#1205](ReactTooltip/react-tooltip#1205) [#1209](#1209) [#1213](#1213)
## [2.1.1](https://github.com/equinor/webviz-subsurface-components/compare/well-log-viewer@2.1.0...well-log-viewer@2.1.1) (2024-10-08) ### Bug Fixes * bump react-tooltip from 5.27.0 to 5.28.0 in /typescript ([#2300](#2300)) ([e355c99](e355c99)), closes [#1209](#1209) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1213](ReactTooltip/react-tooltip#1213) [ReactTooltip/react-tooltip#1211](ReactTooltip/react-tooltip#1211) [ReactTooltip/react-tooltip#1203](ReactTooltip/react-tooltip#1203) [ReactTooltip/react-tooltip#1205](ReactTooltip/react-tooltip#1205) [#1209](#1209) [#1213](#1213)
Addresse #1210
Usage:
Screen.Recording.2024-07-21.at.10.15.51.PM.mov
Summary by CodeRabbit
New Features
disableTooltip
for the Tooltip component, allowing developers to conditionally disable tooltips for specific anchors.disableTooltip
functionality, improving control over tooltip display behavior.Documentation
disableTooltip
function, providing guidance on controlling tooltip visibility based on anchor states.Tests
disableTooltip
prop, ensuring tooltips render correctly based on the prop's return value.