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

Input field losing focus onChange w/ custom renderer #274

Closed
pastudan opened this issue Feb 16, 2019 · 6 comments
Closed

Input field losing focus onChange w/ custom renderer #274

pastudan opened this issue Feb 16, 2019 · 6 comments
Labels
🤷 no/invalid This cannot be acted upon

Comments

@pastudan
Copy link

pastudan commented Feb 16, 2019

I have a custom renderer that generates an input field (below). When the value of the input is changed, it loses focus because the markdown parser seemingly generates new nodes for the input, as well as the parent. What's weird is that React removes and replaces the nodes despite them being exactly the same except for the input's value.

<ReactMarkdown
  source={this.state.content}
  renderers={{
    inlineCode: code => (
      <input
        type="text"
        value={this.state.myInput || code.value}
        onChange={e => {
          this.setState({ myInput: e.target.value })
        }}
      />
    )
  }}
/>

I am sure there's a way to resolve this by adding a key in the correct place, but I was hoping someone might have an idea of where to start looking? Similar issue in #120 but obviously I can't just disable the inputs here.

@angrybacon
Copy link

angrybacon commented Mar 28, 2020

Having the same issue with paragraph where HTML within Markdown will be treated as a paragraph too because of blank lines above and below. Said HTML in some case is a tweet embed component where a placeholder has a fixed height (for when the theme changes) and it waits for a light/dark themed embed. The height is reset to 0 (initial value for my component) every time the renderer below is utilized.

const renderers = {
  /* eslint-disable react/display-name, react/prop-types */
  paragraph: ({ children }) => {
    const component = children[0].type.displayName === 'ParsedHtml' ? 'div' : 'p';
    return <Typography children={children} component={component} gutterBottom />;
  },
  /* eslint-enable react/display-name, react/prop-types */
};

If I test with a constructor alone like this:

const renderers = {
  paragraph: 'div',
};

Or like this:

const renderers = {
  paragraph: Typography,
};

The same node is recycled and my height is left untouched.

Browsing the code, the only difference I can notice has to be in React.createElement itself who must behave differently whether you pass it a proper component versus the function I use in my first code block.

@angrybacon
Copy link

Ah! Found it while posting the above.

/* eslint-disable react/prop-types */
const Paragraph = ({ children }) => {
  const component = children[0].type.displayName === 'ParsedHtml' ? 'div' : 'p';
  return <Typography children={children} component={component} gutterBottom />;
},
/* eslint-enable react/prop-types */

const renderers = {
  paragraph: Paragraph,
};

The renderer simply had to be named. For some reason it doesn't work with shorthand method definitions however.

If someone has

  1. the specifics for how are the renderer cached I'm interested
  2. another way that doesn't involve declaring the renderer (verbose)...

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Oct 8, 2020

@pastudan does @angrybacon's suggestion work for you?


the specifics for how are the renderer cached I'm interested

I'm not sure what you mean here.
It grabs the renderer and runs it:

const renderer = options.renderers[node.type]
// nodes generated by plugins may not have position data
// much of the code after this point will attempt to access properties of the node.position
// this will set the node position to the parent node's position to prevent errors
if (node.position === undefined) {
node.position = (parent.node && parent.node.position) || defaultNodePosition
}
const pos = node.position.start
const key = [node.type, pos.line, pos.column, index].join('-')
if (!ReactIs.isValidElementType(renderer)) {
throw new Error(`Renderer for type \`${node.type}\` not defined or is not renderable`)
}
const nodeProps = getNodeProps(node, key, options, renderer, parent, index)
return React.createElement(
renderer,
nodeProps,
nodeProps.children || resolveChildren() || undefined
)

caching would be based off the key.
this is potentially related to #466
this behavior would also be changed by #428

another way that doesn't involve declaring the renderer

I'm not sure I follow, you want to do custom rendering, without defining a custom renderer?
How would that work?

@wooorm wooorm changed the title Custom Renderer - Input field losing focus onChange Input field losing focus onChange w/ custom renderer Oct 19, 2020
@ChristianMurphy
Copy link
Member

@pastudan friendly reminder that this is still waiting for additional information

@pastudan
Copy link
Author

Unfortunately we removed the code where this was occurring so I can’t test easily. If we do add it back ill give that recommendation a try. Thanks!

@wooorm
Copy link
Member

wooorm commented Oct 20, 2020

I am going to close this as it’s not actionable for us. If someone runs into this again, please open a new issue with e.g., a codesandbox so we can reproduce it!

@wooorm wooorm closed this as completed Oct 20, 2020
@wooorm wooorm added 🤷 no/invalid This cannot be acted upon and removed 🧘 status/waiting labels Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷 no/invalid This cannot be acted upon
Development

No branches or pull requests

4 participants