mdx: In-page hash navigation does not set focus

@silvenon I’d like that yes! Next week sometime?

I tried to put the PR together but getting it to commit is taking more time than I have right now. So sorry. I’m working on a Windows environment and things like the format script is failing for me. Also having line ending hell as all LF gets changed to CRLF when Prettier runs and then it wants to commit all files.

I’m terribly sorry to hear that. When you have time I’d like to pair with you to solve the development environment, because the point of that setup was to minimize the friction of enforcing a code style.

Thanks for writing the docs here, your great! I’ll set you as the author when I’m committing the changes, so it should still show up as your contribution.

@silvenon I tried to put the PR together but getting it to commit is taking more time than I have right now. So sorry. I’m working on a Windows environment and things like the format script is failing for me. Also having line ending hell as all LF gets changed to CRLF when Prettier runs and then it wants to commit all files.

I’d get there eventually but gotta turn attention to other things here right now so would it be OK if I give you the text I was going to add to the Getting Started doc so you can add it?

Here it is:

How it works

MDXProvider uses [React Context][] to provide the component mapping to MDXTag. This way MDXTag knows that it should override the standard components with those provided in this mapping.

Important usage intructions

Because MDXProvider uses React Context directly, it is affected by the same caveats.

It is therefore important that you DON’T declare your components mapping inline in the JSX. Doing so will trigger a rerender of your entire MDX page with every render cycle. Not only is this bad for performance, but it can cause unwanted side affects, like breaking in-page browser navigation.

Avoid this by following the pattern shown above and declare your mapping as a constant.

Updating the mapping object during application runtime

If you need to change the mapping during runtime, declare it on the component’s state object:

import React from 'react'
import { MDXProvider } from '@mdx-js/tag'

import { Heading, Text, Pre, Code, Table } from './components'

export default class Layout extends React.Component {
  state = {
    h1: Heading.H1,
    h2: Heading.H2,
    // ...
    p: Text,
    code: Pre,
    inlineCode: Code
  };
  
  render(){
    const {children, ...props} = this.props;
    return <MDXProvider components={this.state}>
        <main {...props} >
          {children}
          </main>
    </MDXProvider>
  }
}

You can now use the standard setState function to update the mapping object and be assured that it won’t trigger unnecessary renders.

Can probably be fixed with a memoized function deep checking the components prop.

Can it account for potentially new function references too if a given function is defined inline? ala

<MDXProvider components={{
  h1: ({children, ...props}) => <h1>{children}</h1>
}}>

---

nicely done on the digging btw, I think that may be the last piece to get some MDX react-pose staggering I was working on.

No worries! I understand how it goes and it is an awesome tool!

I got a workaround in place for now by manually setting focus inside the Layout component so not dead in the water.

@silvenon Thank you! Will check this out first thing tomorrow. This fix will really help screen reader users a lot.

@ChristopherBiscardi This is great news! Thanks for jumping on this. Looking forward to the release.

@AlmeroSteyn I’ve confirmed that the above layout change works in your repro repo. Please try it and let me know.

@johno I don’t think we need keys (or any other changes I was making) anywhere but this does seem like an awkward footgun in mdx that should be documented at least.