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

Global Config #142

Open
PaulieScanlon opened this issue Sep 24, 2020 · 3 comments
Open

Global Config #142

PaulieScanlon opened this issue Sep 24, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request future release Things that will happen in the futher

Comments

@PaulieScanlon
Copy link
Owner

PaulieScanlon commented Sep 24, 2020

Is your feature request related to a problem? Please describe.
Currently all props for all components are applied inline. eg, every time you use a Tweet component if you wanted to set the theme to "dark" you'd have to do this:

<Tweet tweetLink="..." theme="dark" />

Describe the solution you'd like
Nicky Meuleman created a defaultProps PR in the old repo but i never really got round to reviewing it properly.. apologies Nicky 🙂

I'd like to continue this idea but it'll have to be adapted a bit so that it first works with the core package and via a config prop on the <MDXEmbedProvider config={...}/> and each "sub package" will expose a method for passing options on to the MDXEmbedProvider via the addon/plugin params for each framework, namely Gatsby and Storybook

I'm guessing at this point the new config object would contain keys typed to the interfaces for all components. eg

interface IConfig {
  codepen: ICodePenProps
}

const config = {
  codepen: {
    height: 400,
    tabs: "js"
 }
}: IConfig

And the MDXEmbedProvider would require a new optional prop called config which is typed as IConfig

Config params would mostly likely be provided by the Context API provider and each component would consume the props for the relevant config key.

Inline props should override global config in case more granular control is required on a single instance usage

For addons/plugins i think the usage will look something like this 👇

Gatsby

// gatsby-config.js

module.exports = {
  plugins: [
    ...
   {
     resolve:  `gatsby-plugin-mdx-embed`,
     options: {
       ...
     }
   }
  ]
};

Storybook

// main.js

module.exports = {
  addons: [
    {
      name: 'storybook-addon-mdx-mebed',
      options: {
       ...
      }
    },
  ],
};

And for core props i think the usage will look something like this 👇

Core

// some-file.js

<MDXEmbedProvider config={...}>{children}</MDXEmbedProvider>

Describe alternatives you've considered
There's no alternative at this stage

Additional context
For any addon or plugin we'll need to ensure there are ways to pass options on to MDXEmbedProvider or if not, be sure to call this out in the docs that "Global Config" is not supported.

@PaulieScanlon PaulieScanlon self-assigned this Sep 24, 2020
@PaulieScanlon PaulieScanlon added the enhancement New feature or request label Sep 24, 2020
@PaulieScanlon PaulieScanlon added this to the Stable Release | 1.0.0 milestone Sep 24, 2020
@NickyMeuleman
Copy link
Contributor

NickyMeuleman commented Sep 24, 2020

Great description! Covered all the bases (at least, the ones that I can see).

The user should be able to define a list of default props to use. (eg. all Tweet components have theme="dark" by default)
And, as you noted:

Inline props should override global config in case more granular control is required on a single instance usage

(eg. a Tweet components can be specified with theme="light" inside .mdx will override the default if it was set to theme="dark".)


Without having diven too deep into the source code: This seems like a fine way to implement it! (very similar to that PR I made a while back too, so of course I think that 😁 )

Config params would mostly likely be provided by the Context API provider and each component would consume the props for the relevant config key.

Get the config options into a single place. (eg: the gatsby-config turns into an object of options, the Storybook config also turns into that same object of optiond.)
Then distribute those options to individual components using something like React context.

@PaulieScanlon
Copy link
Owner Author

@NickyMeuleman Hey mate, yeah bang on, I wanted to run this past you because it was your idea. I'll give it first blast and see where i get to and if you have anymore time i'd welcome more feedback. I'll create a PR in the next few days and we can go from there.

@PaulieScanlon PaulieScanlon pinned this issue Sep 24, 2020
@matiasfha
Copy link
Contributor

So.. for the component this will be exposed just as useConfigOptions() hook? Sounds like a good way

@PaulieScanlon PaulieScanlon added the future release Things that will happen in the futher label Oct 26, 2020
@PaulieScanlon PaulieScanlon removed this from the Stable Release | 1.0.0 milestone Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request future release Things that will happen in the futher
Projects
None yet
Development

No branches or pull requests

3 participants