Skip to content

[FEAT REQ] Customizable Arrow Wrapper #929

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

Closed
shahednasser opened this issue Jan 31, 2023 · 8 comments · Fixed by #930 or #935
Closed

[FEAT REQ] Customizable Arrow Wrapper #929

shahednasser opened this issue Jan 31, 2023 · 8 comments · Fixed by #930 or #935

Comments

@shahednasser
Copy link

Is your feature request related to a problem? Please describe.

When using react-tooltip with docusaurus, it results with unexpected behavior. This is because react-tooltip defaults to using div for the tooltip and arrow elements and it results to these div elements being child elements of a p element, which is not allowed in docusaurus.

It's possible to change the wrapper element of the tooltip using the wrapper prop, but it's not possible for the arrow.

Describe the solution you'd like

Allow customizing the element used for the arrow similar to the wrapper prop that customizes the tooltip wrapper element.

Describe alternatives you've considered

When the noArrow prop is set to true, remove the arrow element instead of adding a class that hides it.

@gabrieljablonski
Copy link
Member

Are you able to tell me if using the same wrapper type for the arrow as well would be enough? Such as in #930.

I don't believe there are many use-cases in which it would make sense to have a whole different prop just for choosing the arrow wrapper, so I'd like to avoid it if possible.

@shahednasser
Copy link
Author

I think that can work too. To be honest in our case we didn't need the arrow at all. So, in our case it's fine to use the same element type as the wrapper.

@gabrieljablonski
Copy link
Member

@shahednasser Please try out v5.7.3 and let us know.

@shahednasser
Copy link
Author

Thanks! Will test it out soon @gabrieljablonski

@shahednasser
Copy link
Author

Update: issue still occurring even with this fix. Interesting thing is that I still see this warning in the console during development:

Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>.

Seems like on initial render the element is div and then switches to span?

For some context: this issue is causing code blocks (which is where we were using the react-tooltip component) disappear after building and serving the docusaurus website. With this fix, the code blocks are visible when navigating the website, but on opening a page directly in the browser, the code blocks are removed from the page.

@gabrieljablonski
Copy link
Member

Are you able to provide a basic sample project with your current setup so I can reproduce the issue?

@shahednasser
Copy link
Author

shahednasser commented Feb 3, 2023

Our documentation is available on our monorepo under www/docs. To reproduce:

  1. Install react-tooltip.
  2. Replace the content of the src/theme/Tooltip/index.js with something like this:
import React, { useState, useEffect } from 'react';
import { Tooltip as ReactTooltip } from 'react-tooltip'
import uuid from 'react-uuid';
import 'react-tooltip/dist/react-tooltip.css'

export default function Tooltip ({ children, text, tooltipClassName }) {
  const [elementId, setElementId] = useState(null)

  useEffect(() => {
    if (!elementId) {
      setElementId(uuid())
    }
  }, [elementId])

  return (
    <>
      <span 
        id={elementId}
        data-tooltip-content={text}
      >
        {children}
      </span>
      <ReactTooltip anchorId={elementId} className={tooltipClassName} />
    </>
  );
};
  1. Run yarn start. When the docusaurus website runs, go to localhost:3000/advanced/backend/upgrade-guides/medusa-core/1-7-3/. There should be no issues but the warning mentioned earlier will be visible in the console.
  2. If you run the build command:
NODE_OPTIONS="--max-old-space-size=8192" yarn build

Then serve:

yarn serve

And try going to the same page, the code block disappears.

@danielbarion
Copy link
Member

danielbarion commented Feb 3, 2023

@shahednasser I'll submit a PR to Medusa project in a few mins, I tested both builds and the next release will work as expected.

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