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

Migration to v3 from v1 #13

Closed
hudochenkov opened this issue Nov 13, 2017 · 14 comments
Closed

Migration to v3 from v1 #13

hudochenkov opened this issue Nov 13, 2017 · 14 comments
Labels

Comments

@hudochenkov
Copy link

Currently we use [email protected]. We have <ReactHint /> in top container component to avoid conflicts with multiple instances in v1. Half of our components use data-rh-cls. Is there a strategy how to migrate to v3 when only half of components uses data-rh-cls?

Is this possible to get back data-rh-cls back, which was deprecated in 2.0.0?

Thank you for your library!

@slmgc
Copy link
Owner

slmgc commented Nov 13, 2017

Is it the same class name value for data-rh-cls for half of components? If it is, then you can define a second instance of <ReactHint /> which will handle this kind of tooltips:

/*
	This instance handles the default tooltips.
*/
<ReactHint events delay={100} />

/* 
	This instance handles the new tooltips.
*/
<ReactHint
	/*
		Re-define the default `data-rh` tooltip attribute
		name to distinguish between two tooltip types.
	*/
	attribute="data-custom-attribute-name"

	/*
		Set the default class name to be the same
		as for `data-rh-cls`.
	*/
	className="custom-class-name"

	/* 
		Set the show delay to match the default value
		for react-hint v.1.
	*/
	delay={100}

	/* 
		Don't forget to enable events. You can specify
		which types of events trigger the tooltip, e.g.
		click, focus, hover.
	*/
	events={{hover: true, click: false}}

	/*
		Add this property in case you want to retain
		the usual behaviour, so the tooltip won't hide
		if you hover over it.
	*/
	persist
/>

...

/*
	This element will trigger the default tooltip.
*/
<div data-rh="Default tooltip" />

/*
	This element will trigger the new tooltip with
	the custom styling.
*/
<div data-custom-attribute-name="New tooltip" />

Does it solve your case? If not, we'll figure it out :)

@slmgc slmgc added the question label Nov 13, 2017
@hudochenkov
Copy link
Author

Thank you for a quick reply!

Your solution works! As you assumed we use the same class name for one half of components.

I faced two problems while adopting v3:

  1. event should be enabled, otherwise no tooltips :)
  2. Custom class name makes impossible to not break BEM if custom class was used as modifier in v1. We're using .react-hint--tip in data-rh-cls to modify default .react-hint styles. In v3 custom class replaces react-hint in all class names which this component generates. I came up with this class name as a workaround to have same CSS as before: react-hint--tip react-hint. But it generates incorrect BEM markup:
<div class="react-hint--tip react-hint react-hint--tip react-hint--top">
	<div class="react-hint--tip react-hint__content">New tooltip</div>
</div>

It's not a big problem for us, but I though you would like to know.

@slmgc
Copy link
Owner

slmgc commented Nov 14, 2017

Hm, I see your point. I guess this can be solved by splitting the class name by whitespace and attaching a postfix to each of the resulting classes, e.g.

<div class="react-hint--tip react-hint react-hint--tip--top react-hint--top">
	<div class="react-hint--tip__content react-hint__content">New tooltip</div>
</div>

Do you think this will be better? I am open to proposals if you have any.

@hudochenkov
Copy link
Author

react-hint--tip__content still is not correct BEM class name.

Current implementation allows to rename block. With class name block-name:

<div class="block-name block-name--top">
	<div class="block-name__content">New tooltip</div>
</div>

And it's good. It solve namespace problem.

I think you shouldn't change anything. With following code I could achieve desired results (not ideal, but works :)):

<ReactHint
    className="react-hint--tip react-hint"
    onRenderContent={(target, content) => (
        <div className="react-hint__content">{content}</div>
    )}
/>
<div class="react-hint--tip react-hint react-hint--tip react-hint--top">
	<div class="react-hint__content">New tooltip</div>
</div>

Thank you for your help!

@davidrenne
Copy link

davidrenne commented Oct 16, 2018

I am so lost as to how to do two custom rendered react hint's

Whenever I hover over my container to pop up the hint object, both of my tooltips show at the same time and start showing my videos in my custom render.

It seems I am stuck for the evening with 1 tool tip per page until I can figure out how the heck these custom classes go together so that one container is isolated to it's own thing.

Here's some files to help you understand how I am using it:

Callers:

<ToolTipVideo tipTitle={<h3 style={{textAlign: "center", margin: "0 0 1.5em", fontSize: 20}}>{window.appContent.GatewayActivation}</h3>} tipVideo='/web/app/images/videos/03%20-%20License%20Activation.mp4'/>

<ToolTipVideo tipTitle={<h3 style={{textAlign: "center", margin: "0 0 1.5em", fontSize: 20}}>{window.pageContent.WhatisVelocity}</h3>} tipVideo='/web/app/images/videos/01%20-%20What%20is%20Velocity.mp4'/>

ToolTipVideo class:

import React, {Component} from 'react';
import BaseComponent from './base';
import HelpIcon from "material-ui/svg-icons/action/help";
import ReactPlayer from 'react-player';
import {ToolTipHint} from "../globals/forms";

class ToolTipVideo extends BaseComponent {
  constructor(props, context) {
    super(props, context);
    this.state = {playing: true};
  }

  componentWillUnmount() {
    this.setState((s) => {
      s.playing = false;
      return s;
    })
  }


  render() {
    try {
      let tipStyle = {position:"absolute", zIndex: 999999, padding: 50, backgroundColor: "white", border: "2px solid #ebebeb"};
      if (this.props.tipStyle != undefined) {
        this.globs.forEach(this.props.tipStyle, (v, k) => {
          tipStyle[k] = v;
        });
      }

      return <ToolTipHint tipId={this.props.tipVideo.replace(/[^a-z0-9]/gi,'')} eventType={{click: true, hover: true}} tip={(target, content) => {
            return <span style={tipStyle}>
            {this.props.tipTitle}
            <ReactPlayer controls={true} url={this.props.tipVideo} playing={this.state.playing}/>
          </span>;
        }}
      />;
    } catch(e) {
      return this.globs.ComponentError("ToolTipCSS", e.message, e);
    }
  }
}

ToolTipVideo.propTypes = {
  reactHintProps: React.PropTypes.any,
  tipStyle: React.PropTypes.any,
  tipTitle: React.PropTypes.node,
  tipVideo: React.PropTypes.string.isRequired
};

export default ToolTipVideo;

And the ReactHint <ToolTipHint/> abstraction library. Note I added a tipId prop to pass a dynamically unique name of classes hoping that this would help out in making them so they would not open up both tips at the same time. But I just need some help in understanding how you have this wired up and what small adjustments I can try out in my implementation.

import React, {Component} from 'react';
import BaseComponent from './base';
import ReactHintFactory from 'react-hint'
import 'react-hint/css/index.css'
const ReactHint = ReactHintFactory(React)
import HelpIcon from "material-ui/svg-icons/action/help";

class ToolTipHint extends BaseComponent {
  constructor(props, context) {
    super(props, context);
  }

  render() {
    try {

      let helpData = {};
      if (this.globs.typeof(this.props.tip) == "Function") {
        helpData["data-custom"] = true;
      } else {
        helpData["data-rh"] = this.props.tip;
        helpData["data-rh-at"] = this.props.position;
      }


      let childrenWithProps = null;

      if (this.props.helpNodeIcon != undefined) {
        childrenWithProps = React.cloneElement(this.props.helpNodeIcon, helpData);
      }

      let helpStyles = {height: this.props.height};
      if (this.props.helpStyles != undefined) {
        this.globs.forEach(this.props.helpStyles, (v, k) => {
          helpStyles[k] = v;
        });
      }

      let tipId = "";
      if (this.props.tipId != undefined) {
        tipId = this.props.tipId;
      } else if ( this.globs.typeof(this.props.tip) != "Function") {
        tipId = this.props.tip.replace(/[^a-z0-9]/gi,'')
      }

      let helpIcon = this.props.helpNodeIcon != undefined ? childrenWithProps : <span {...helpData}><HelpIcon color={window.materialColors.blue600} style={helpStyles}/></span>;
      let reactHint = this.globs.typeof(this.props.tip) == "Function" ? <ReactHint
      persist
      attribute={"data-custom"}
      autoPosition
      delay={40}
      className={tipId + "-hint"}
      events={this.props.eventType}
      onRenderContent={(target, content) => {
        return <div className={tipId + "-hint__content"}>
          {this.props.tip(target, content)}
        </div>
      }}/> : <ReactHint className={tipId + "-hint font-small-rh"} autoPosition events={this.props.eventType} delay={100} />;

      return <span style={{cursor: this.props.cursor}}>
        {helpIcon}
        {reactHint}
      </span>;
    } catch(e) {
      return this.globs.ComponentError("ToolTip", e.message, e);
    }
  }
}


/*
Example using custom tip render and custom help icon which needs to be clicked
  <ToolTipHint eventType={{click: true}} helpNodeIcon={<span>dave</span>} tip={(target, content) => {
    session_functions.VelocityDump(target, content);
        return <span><button>Ok</button></span>;
    }}
  />*/
ToolTipHint.propTypes = {
  tip: React.PropTypes.any,
  tipId: React.PropTypes.string,
  position: React.PropTypes.string,
  height: React.PropTypes.string,
  cursor: React.PropTypes.string,
  helpNodeIcon: React.PropTypes.node,
  helpStyles: React.PropTypes.object,
  eventType: React.PropTypes.object
};

ToolTipHint.defaultProps = {
  height: 19,
  position: "right",
  cursor: "help",
  eventType: {hover: true}
};

export default ToolTipHint;

Here are the two divs hovered over at the same time.

Start:

screenshot 2018-10-16 16 33 32

As hovering over the top help icon (the videos both run and both divs are showing up... if you notice the two grey boxes those are two instances of a video object but with different videos popping up when I hover over one):

screenshot 2018-10-16 16 31 45

FYI this library will be used in our help videos for Atlona velocity, but only if I can figure out how to load multiple tips in one page without all of them opening up at the same time. (Desperate for help to get this project out of my hair so I can be onto more interesting things than hover popups!) LOL!

Overall I think once I understand how these custom classes work, I will be sending a pull request with some better documentation.

@davidrenne
Copy link

@slmgc sorry to comment on a closed issue. I can add as a separate issue if you'd like?

@slmgc
Copy link
Owner

slmgc commented Oct 16, 2018

Hi @davidrenne, I am really sorry to hear that you have so much trouble dealing with multiple ReactHint instances. I should have updated the docs a long time ago... 😓

I believe the issues is that every wrapper instance uses the same hard-coded attribute name "data-custom". Thus, all of them listen to the same events, basically.

If you'll check my topmost comment here: #13 (comment), it describes what needs to be done there, let me quote myself:

<ReactHint
	/*
		Re-define the default `data-rh` tooltip attribute
		name to distinguish between two tooltip types.
	*/
	attribute="data-custom-attribute-name"

So, all you need to do is to define a unique attribute per ReactHint instance like this:

<ReactHint attribute="data-custom-attribute-name-for-the-first-instance"
	...
/>

/* If it's really required to show completely different-looking tooltips
for some edge cases, you define a second instance */

<ReactHint attribute="data-custom-attribute-name-for-the-second-instance"
	...
/>

Though, it will solve your issue, I believe there might be a misunderstanding of how react-hint works. <ReactHint /> is supposed to be a singleton in 99.99% of cases. You don't wrap your components in <ReactHint /> or, in your case, in <ToolTipHint />. Correct me if I am wrong, from what I can see, it seems that your case doesn't require multiple instances of ReactHint as tooltips look the same.

Hope this helps!

@davidrenne
Copy link

@slmgc u rock! ill check it out in the morning and see if I can get it working.

Yea I think if I have 1 node of reacthint maybe ill have less issues.

Thanks for a quick response. Ill let you know if i get it working tomorrow.

@davidrenne
Copy link

@slmgc Yea I just dont get how it can be a singleton node.

You pass a function onRenderContent for a custom render to render the inside of the div when the popup shows.

I did actually try to change the attribute={} prop to a unique one, but whenever I change it to anything but "data-custom" the help icons disappear on me.

I am not sure how multiple nodes showing different videos would actually be able to pass different functions for a custom Render.

I think I may need to rethink and not use onRenderContent because I dont understand how onRenderContent can be used to show different things. I need to go back to the drawing board and output a render through inline HTML somehow and referencing those classNames somehow into the singleton.

I still am lost as to how can you have two different tool tips with a single node? How is it possible to have two nodes with two different classes? Should a developer somehow set state as someone hovers over their hover node to change the attribute prop to what HTML node it is supposed to show?

@davidrenne
Copy link

davidrenne commented Oct 17, 2018

@slmgc

I think I got it figured out. I think my case is the 0.01 case because each popup has unique content onRender.

When my tip is a function I merely generate a unique tip ID from the video URL

      if (this.globs.typeof(this.props.tip) == "Function") {
        helpData["data-custom-popup-" + tipId] = tipId;
      } else {
        helpData["data-rh"] = this.props.tip;
        helpData["data-rh-at"] = this.props.position;
      }

It probably never showed up because I was changing just the attribute inside of the ReactHint and not adjusting the above block of code to match it.

Here's the full file which is not isolating each popup properly.

import React, {Component} from 'react';
import BaseComponent from './base';
import ReactHintFactory from 'react-hint'
import 'react-hint/css/index.css'
const ReactHint = ReactHintFactory(React)
import HelpIcon from "material-ui/svg-icons/action/help";

class ToolTipHint extends BaseComponent {
  constructor(props, context) {
    super(props, context);
  }

  render() {
    try {

      let helpData = {};

      let tipId = "";
      if (this.props.tipId != undefined) {
        tipId = this.props.tipId;
      } else if ( this.globs.typeof(this.props.tip) != "Function") {
        tipId = this.props.tip.replace(/[^a-z0-9]/gi,'')
      }

      if (this.globs.typeof(this.props.tip) == "Function") {
        helpData["data-custom-popup-" + tipId] = tipId;
      } else {
        helpData["data-rh"] = this.props.tip;
        helpData["data-rh-at"] = this.props.position;
      }


      let childrenWithProps = null;

      if (this.props.helpNodeIcon != undefined) {
        childrenWithProps = React.cloneElement(this.props.helpNodeIcon, helpData);
      }

      let helpStyles = {height: this.props.height};
      if (this.props.helpStyles != undefined) {
        this.globs.forEach(this.props.helpStyles, (v, k) => {
          helpStyles[k] = v;
        });
      }
      let helpIcon = this.props.helpNodeIcon != undefined ? childrenWithProps : <span {...helpData}><HelpIcon color={window.materialColors.blue600} style={helpStyles}/></span>;
      let reactHint = this.globs.typeof(this.props.tip) == "Function" ? <ReactHint
      persist
      attribute={"data-custom-popup-" + tipId}
      autoPosition
      delay={40}
      className={tipId + "-hint"}
      events={this.props.eventType}
      onRenderContent={(target, content) => {
        return <div className={tipId + "-hint__content"}>
          {this.props.tip(target, content)}
        </div>
      }}/> : <ReactHint className={tipId + "-hint font-small-rh"} autoPosition events={this.props.eventType} delay={100} />;

      return <span style={{cursor: this.props.cursor}}>
        {helpIcon}
        {reactHint}
      </span>;
    } catch(e) {
      return this.globs.ComponentError("ToolTip", e.message, e);
    }
  }
}


/*
Example using custom tip render and custom help icon which needs to be clicked
  <ToolTipHint eventType={{click: true}} helpNodeIcon={<span>dave</span>} tip={(target, content) => {
    session_functions.VelocityDump(target, content);
        return <span><button>Ok</button></span>;
    }}
  />*/
ToolTipHint.propTypes = {
  tip: React.PropTypes.any,
  tipId: React.PropTypes.string,
  position: React.PropTypes.string,
  height: React.PropTypes.string,
  cursor: React.PropTypes.string,
  helpNodeIcon: React.PropTypes.node,
  helpStyles: React.PropTypes.object,
  eventType: React.PropTypes.object
};

ToolTipHint.defaultProps = {
  height: 19,
  position: "right",
  cursor: "help",
  eventType: {hover: true}
};

export default ToolTipHint;

@slmgc
Copy link
Owner

slmgc commented Oct 18, 2018

@davidrenne I still don't get it when you say "I think my case is the 0.01 case because each popup has unique content onRender." Does it mean every tooltip looks completely different with custom css styling? Or is it about custom content only? Because if it's about content with the same-looking tooltip, then it means that you are probably overcomlicating things.

"Yea I just dont get how it can be a singleton node." Have you tried using a single ReactHint instance without adding your custom wrapper? You can easily override onRenderContent with a custom content without doing this.

@davidrenne
Copy link

Yea I think i am overcomplicating, but it works right now with spawning off a unique node or attribute for each popup with a different video of content using a different onRenderContent.

Thanks for listening and helping me out! Nice library which floats in a nice position each time with minimal changes to styling.

@slmgc
Copy link
Owner

slmgc commented Oct 19, 2018

I'll try to come up with an example for you to be used with your video tooltips, maybe it will help you as creating a new ReactHint instance per tooltip is costly, this is the reason why this library was created in the first place, to optimize things for such cases and that's why it should be a singleton in most of the cases.

@davidrenne
Copy link

That would be delicious. So far performance wise, its like instantaneous for my small use case of a video tag. I do know why probably you went into the singleton direction. Node rendering seems to be most CPU intensive the more nodes you have on re-renders. Like for example SVG's we found are super slow compared to a simple img tag with a png. But you lose the benefits of color controls etc.

Thanks for showing me the light on my example as to how I would implement it and I can try it out in our project once you guide me a bit.

To me, it was just confusing as to how to interface and inject new videos into the tree of the rendered popup.

Thanks for being awesome @slmgc

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

No branches or pull requests

3 participants