🪟 🎨 Update Tooltip component to match design library #14816

edmundito · 2022-07-18T22:15:11Z

What

Closes #14581

Updates the Tooltip component to match the design library. Adds the following:

Light and Dark Mode
Align to top, right, bottom, left side of the parent element
Add components to match design layout: <TooltipLearnMoreButton />, <TooltipTable />

https://www.loom.com/share/c375493ede804ddb9e28abf06340a0b3

How

The component now uses floating-ui to set the tooltip position. There is a specific timing effect to ensure that the user can hover over the tooltip to click on links in the tooltip.
The component is now migrated to SCSS
There was an InfoIcon component nested inside the ToolTip component. It is now moved to components/icons
Added storybook for testing

Tests

Tested every component that used the Tooltip component to verify that it looks good.

edmundito · 2022-07-18T22:16:22Z

airbyte-webapp/.storybook/withProvider.tsx

@@ -50,7 +50,7 @@ export const withProviders = (getStory) => (
                  providers={[]}
                  >
                  <DocumentationPanelProvider>
-                    <FeatureService>
+                    <FeatureService features={[]}>


Broken by #14559

airbyte-webapp/package.json

airbyte-webapp/src/scss/_variables.scss

dizel852

Wasn't able to test locally - storybook crashes.
@edmundito how have you managed to run storybook?

dizel852 · 2022-08-02T13:23:50Z

Wasn't able to test locally - storybook crashes. @edmundito how have you managed to run storybook?

Just checked one more time - now it's okay!

edmundito · 2022-08-04T16:51:05Z

airbyte-webapp/src/components/ReleaseStageBadge/ReleaseStageBadge.tsx

@@ -41,7 +41,7 @@ export const ReleaseStageBadge: React.FC<ReleaseStageBadgeProps> = ({ stage, sma
    );

  return tooltip ? (
-    <ToolTip control={badge} cursor="help">


While reviewing the design with @Upmitt, it was decided that it didn't make sense to show the help cursor here.

…ht mode

* Add react-tether dependency * Update ToolTip to use react-tether * Add align property to ToolTip

Update tooltip corner to 5px Update package-lock with react-tether

* Add tooltip context * Update tooltip stories to demo with new components

airbyte-webapp/src/views/Connection/ConnectionForm/components/InformationToolTip.tsx

lmossman

Just had a couple questions when reading through this!

lmossman · 2022-08-10T17:39:03Z

airbyte-webapp/src/components/ToolTip/TooltipLearnMoreLink.tsx

+  url: string;
+}
+
+export const TooltipLearnMoreLink: React.VFC<TooltipLearnMoreLinkProps> = ({ url }) => (


Just curious, why did you use React.VFC<> here, but React.FC<> in the ToolTip component?

VFC (Void FunctionComponent) is a type of component that does not allow children to be passed through it. It's something I discovered recently while working on this.

Ah I see, and we don't want this component to have any children because it is just a simple div with a link

lmossman · 2022-08-10T17:49:33Z

airbyte-webapp/src/components/ToolTip/TooltipTable.tsx

+}
+
+export const TooltipTable: React.VFC<TooltipTableProps> = ({ rows }) => {
+  const { theme } = useTooltipContext();


For my own learning, could you briefly explain why you chose to use a context here? Is this because the theme prop could not be passed directly to TooltipTable, because that sub-component is not always present?

I did this in order to understand the context of the tooltip without having to pass the props twice. Otherwise in the component that implements you you'd have to pass the same theme:

<Tooltip theme="light"> <TooltipTable theme="light" /> </Tooltip> // Nothing stopping you from doing: <Tooltip theme="light"> <TooltipTable /> // If you don't pass the theme and have no context, the theme of the table will be "dark" </Tooltip>

Ah okay, so this is basically forcing the TooltipTable to match the same theme as its parent Tooltip through the use of context, removing that burden from the developer who adds the TooltipTable component somewhere. That is cool, thanks for explaining!

krishnaglick · 2022-08-10T19:07:39Z

airbyte-webapp/src/components/ToolTip/TooltipLearnMoreLink.tsx

+
+export const TooltipLearnMoreLink: React.VFC<TooltipLearnMoreLinkProps> = ({ url }) => (
+  <div className={styles.container}>
+    <a href={url} target="_blank" rel="noreferrer">


Probably not worth the effort, nor would this be the place, but I'd love to have a component that would always tack on target="_blank" rel="noreferrer"
But remembering to use it would likely be more obnoxious than just adding this since the linter yells about it.
If we ever wanted click tracking for analytics we could create a TrackingLink component or something and put that in there. 🤷

airbyte-webapp/src/components/ToolTip/TooltipTable.tsx

krishnaglick · 2022-08-10T19:12:45Z

airbyte-webapp/src/components/ToolTip/context.ts

+
+import { TooltipContext } from "./types";
+
+export const tooltipContext = createContext<TooltipContext | null>(null);


We use both camelCase and PascalCase for context's. We should decide one way or the other just for consistency.
My argument for PascalCase is since it's used as part of JSX and looks like a component, it should match that kind of usage.

airbyte-webapp/src/components/ToolTip/index.ts

airbyte-webapp/src/views/Connection/ConnectionForm/components/InformationToolTip.tsx

krishnaglick · 2022-08-10T19:19:28Z

airbyte-webapp/src/components/ToolTip/types.ts

+  placement?: Placement;
+}
+
+export type TooltipContext = ToolTipProps;


Feels odd to look at, but I get it.

dizel852

Overall - LGTM.
Tested locally using storybook

* Add ToolTip storybook * Update ToolTip to scss * gitignore storybook-static dir * Move InfoIcon from ToolTip to icons and remove duplicate * Fix disabled class name in ToolTip * Update ToolTip colors and spacing to match updated design and add light mode * Update ToolTip to show from any side perfectly centered * Add react-tether dependency * Update ToolTip to use react-tether * Add align property to ToolTip * Add link colors to tooltip links and update storybook * Fix font-size in Tooltip * Add minor tweaks to Tooltip component * Remove "help" cursor from state badge Update tooltip corner to 5px Update package-lock with react-tether * Tooltip mode -> Tooltip theme * Add tooltip helper components for learn more url and table * Add tooltip context * Update tooltip stories to demo with new components * REname tooltip index from tsx to ts * Update tooltip to use floating-ui instead of react-tether * Move z-index values to own z-indices file * Update InformationTooltip to use regular tooltip style

edmundito added area/frontend area/frontend Related to the Airbyte webapp ui/components labels Jul 18, 2022

github-actions bot added the area/platform issues related to the platform label Jul 18, 2022

edmundito commented Jul 18, 2022

View reviewed changes

edmundito changed the title ~~Update Tooltip component to match design library~~ 🪟 🎨 Update Tooltip component to match design library Jul 18, 2022

edmundito force-pushed the edmundito/update-tooltip branch 2 times, most recently from 38eeb8f to feb3d8b Compare July 22, 2022 21:51

edmundito marked this pull request as ready for review July 22, 2022 22:36

edmundito requested a review from a team as a code owner July 22, 2022 22:36

timroes reviewed Jul 28, 2022

View reviewed changes

airbyte-webapp/package.json Outdated Show resolved Hide resolved

timroes reviewed Jul 28, 2022

View reviewed changes

airbyte-webapp/src/scss/_variables.scss Outdated Show resolved Hide resolved

dizel852 requested changes Aug 1, 2022

View reviewed changes

edmundito force-pushed the edmundito/update-tooltip branch 2 times, most recently from e8754f9 to 729abd4 Compare August 3, 2022 19:43

edmundito requested review from dizel852 and timroes August 3, 2022 19:45

matter-q mentioned this pull request Aug 4, 2022

🪟 🎉 Move the cancel button outside the run click area #14955

Merged

edmundito commented Aug 4, 2022

View reviewed changes

edmundito force-pushed the edmundito/update-tooltip branch from 21ab0e2 to 0287dea Compare August 4, 2022 18:15

edmundito requested a review from a team August 9, 2022 13:36

edmundito added 8 commits August 10, 2022 12:32

Add ToolTip storybook

8f77b12

Update ToolTip to scss

176874c

gitignore storybook-static dir

9052dda

Move InfoIcon from ToolTip to icons and remove duplicate

d168b82

Fix disabled class name in ToolTip

230895f

Update ToolTip colors and spacing to match updated design and add lig…

a5769b9

…ht mode

Update ToolTip to show from any side perfectly centered

c043fad

* Add react-tether dependency * Update ToolTip to use react-tether * Add align property to ToolTip

Add link colors to tooltip links and update storybook

3055b9b

edmundito added 8 commits August 10, 2022 12:32

Fix font-size in Tooltip

b2a1a69

Add minor tweaks to Tooltip component

e80fc63

Remove "help" cursor from state badge

f4af838

Update tooltip corner to 5px Update package-lock with react-tether

Tooltip mode -> Tooltip theme

00ca040

Add tooltip helper components for learn more url and table

3ec31ef

* Add tooltip context * Update tooltip stories to demo with new components

REname tooltip index from tsx to ts

cb2aa14

Update tooltip to use floating-ui instead of react-tether

b8d7666

Move z-index values to own z-indices file

111e072

edmundito force-pushed the edmundito/update-tooltip branch from 0287dea to 111e072 Compare August 10, 2022 16:32

edmundito commented Aug 10, 2022

View reviewed changes

airbyte-webapp/src/views/Connection/ConnectionForm/components/InformationToolTip.tsx Outdated Show resolved Hide resolved

Update InformationTooltip to use regular tooltip style

f211b87

lmossman reviewed Aug 10, 2022

View reviewed changes

krishnaglick reviewed Aug 10, 2022

View reviewed changes

dizel852 approved these changes Aug 17, 2022

View reviewed changes

edmundito merged commit e380c26 into master Aug 18, 2022

edmundito deleted the edmundito/update-tooltip branch August 18, 2022 18:05

octavia-squidington-iii mentioned this pull request Aug 19, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1-alpha #15775

Closed

evantahler mentioned this pull request Aug 22, 2022

Bump Airbyte version from 0.40.0-alpha to 0.40.1 #15857

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎨 Update Tooltip component to match design library #14816

🪟 🎨 Update Tooltip component to match design library #14816

edmundito commented Jul 18, 2022 •

edited

Loading

edmundito Jul 18, 2022

dizel852 left a comment

dizel852 commented Aug 2, 2022

edmundito Aug 4, 2022 •

edited

Loading

lmossman left a comment

lmossman Aug 10, 2022

edmundito Aug 10, 2022

lmossman Aug 10, 2022

lmossman Aug 10, 2022

edmundito Aug 10, 2022

lmossman Aug 10, 2022 •

edited

Loading

krishnaglick Aug 10, 2022

krishnaglick Aug 10, 2022

krishnaglick Aug 10, 2022

dizel852 left a comment


		import { TooltipContext } from "./types";

		export const tooltipContext = createContext<TooltipContext \| null>(null);

🪟 🎨 Update Tooltip component to match design library #14816

🪟 🎨 Update Tooltip component to match design library #14816

Conversation

edmundito commented Jul 18, 2022 • edited Loading

What

How

Recommended reading order

Tests

Choose a reason for hiding this comment

dizel852 left a comment

Choose a reason for hiding this comment

dizel852 commented Aug 2, 2022

edmundito Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

lmossman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmossman Aug 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dizel852 left a comment

Choose a reason for hiding this comment

edmundito commented Jul 18, 2022 •

edited

Loading

edmundito Aug 4, 2022 •

edited

Loading

lmossman Aug 10, 2022 •

edited

Loading