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

Tooltip: User defined id for aria-describedBy #1561

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

harish-prakash
Copy link
Contributor

@harish-prakash harish-prakash commented Dec 1, 2022

Overview
Using the tooltip in SSR (server-side rendering) applications cause an ugly error due to render comparison.

Warning: Prop `aria-describedby` did not match. Server: "evergreen-tooltip-40" Client: "evergreen-tooltip-2"

Documentation

  • Updated Typescript types and/or component PropTypes
  • Added / modified component docs

@netlify
Copy link

netlify bot commented Dec 1, 2022

Deploy Preview for evergreen-storybook ready!

Name Link
🔨 Latest commit 8e5735e
🔍 Latest deploy log https://app.netlify.com/sites/evergreen-storybook/deploys/638e30bbf39664000810f657
😎 Deploy Preview https://deploy-preview-1561--evergreen-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -22,7 +22,7 @@ const Tooltip = memo(function Tooltip(props) {
statelessProps = emptyProps
} = props

const id = useId('evergreen-tooltip')
const id = useId('evergreen-tooltip', props.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I pulled this down and it seems to solve the warning in the original issue (when providing an explicit id). However, I think this is somewhat of a workaround - ideally, the useId hook should be reworked to return stable ids between the client <> server so an explicit id isn't required to suppress this warning during SSR.

I'm cool w/ adding in the ability to set an explicit id for the Tooltip either way, but I think we'll need to add it to TooltipProps in the index.d.ts file for TypeScript users:

evergreen/index.d.ts

Lines 3109 to 3142 in 0c36c45

export interface TooltipProps {
/**
* The appearance of the Tooltip.
*/
appearance?: TooltipAppearance
/**
* The position the Tooltip is on.
*/
position?: PositionTypes
/**
* The content of the Tooltip.
*/
content: React.ReactNode
/**
* Time in ms before hiding the Tooltip.
*/
hideDelay?: number
/**
* Time in ms before showing the Tooltip.
*/
showDelay?: number
/**
* Controls whether the Tooltip is shown or not.
* - When `true`, the component is always shown, regardless of the whether the target is hovered.
* - When `false`, the component is never shown, regardless of the whether the target is hovered.
* - When `undefined`, the component is uncontrolled and the isShown state is handled internally
* (i.e. the Tooltip is shown when the target is hovered)
*/
isShown?: boolean
/**
* Properties passed through to the Tooltip.
*/
statelessProps?: PolymorphicBoxProps<'div', TooltipStatelessProps>
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but I think we'll need to add it to TooltipProps in the index.d.ts file for TypeScript users

🤔 I was under the impression that index.d.ts was auto-generated, oh well 🤷‍♂️ more to learn...

ideally, the useId hook should be reworked to return stable ids between the client <> server

not that I intend to do it or particularly in this PR but I am curious, how would you do that? Without a unique reference for this component between the server and client, how would you create a stable id?

Copy link
Contributor

Choose a reason for hiding this comment

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

index.d.ts "should" be generated, but we're still on JS 🥲

Without a unique reference for this component between the server and client, how would you create a stable id?

I'm not entirely sure! I'm not super familiar with SSR myself, but I did some quick searching and it looks like useId is available as a hook directly from React in v18 which solves this: reactwg/react-18#111

Mantine implements a stable id hook by setting the state in an effect:

import React, { useState } from 'react';
import { useIsomorphicEffect } from '../use-isomorphic-effect/use-isomorphic-effect';


const randomId = () => `mantine-${Math.random().toString(36).slice(2, 11)}`;


const useReactId: () => string | undefined =
  (React as any)['useId'.toString()] || (() => undefined);


function useClientId() {
  const [uuid, setUuid] = useState('');


  useIsomorphicEffect(() => {
    setUuid(randomId());
  }, []);


  return uuid;
}


function getReactId() {
  const id = useReactId();
  return id ? `mantine-${id.replace(/:/g, '')}` : '';
}


export function useId(staticId?: string) {
  return typeof staticId === 'string' ? staticId : getReactId() || useClientId();
}

index.d.ts Outdated Show resolved Hide resolved
@brandongregoryscott brandongregoryscott merged commit 4fd5720 into segmentio:master Dec 5, 2022
@harish-prakash
Copy link
Contributor Author

Thank you for handling this 🙇‍♂️

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

Successfully merging this pull request may close these issues.

NextJS Warning: Prop aria-describedby did not match.
2 participants