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

Extensions API #354

Open
okonet opened this Issue Mar 15, 2017 · 37 comments

Comments

Projects
None yet
5 participants
@okonet
Copy link
Member

okonet commented Mar 15, 2017

We might want to take a look at https://github.com/camwest/react-slot-fill for allowing extensions for SG.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Mar 15, 2017

We were talking about Storybook like plugins that would add tabs under examples. But this also can be useful.

@sapegin sapegin added the enhancement label Mar 15, 2017

@okonet

This comment has been minimized.

Copy link
Member

okonet commented Mar 15, 2017

Yeah, I actually don't like how storybook does it since it's very restrictive. This one should theoretically allow writing custom visualizers for the components and create custom controlled components (like knobs?). We should try it out and see if it works well for us.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Mar 15, 2017

I think for knobs Storybook-style is quite good and enough. Why do you think it won’t work well?

@okonet

This comment has been minimized.

Copy link
Member

okonet commented Mar 15, 2017

My concern is more UI-wise: storybook only allow you to extend by creating tabs around the content there we are interested in creating extension inside of the content.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Mar 15, 2017

Could you make some mockup of your ideas? That would really help ;-)

@okonet

This comment has been minimized.

Copy link
Member

okonet commented Mar 15, 2017

I could when I get some more time :(

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Mar 17, 2017

Have watched a ReactConf talk about it and this is amazing! But looks like it only works with Fiber. Not sure they will / can support React 15.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Mar 17, 2017

What do we want to extend by the way?

  1. Tabs under the component playground (Storybook like) seems useful: non-editable code, snapshots, knobs…
  2. Access to style guide config.

What else?

@MicheleBertoli

This comment has been minimized.

Copy link
Member

MicheleBertoli commented Apr 23, 2017

+1 for tabs, it seems a great starting point to me.

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented Apr 23, 2017

I agree with @okonet, I would love have both possibilities:

  • add isolated features/plugins in tabs (around content)
  • extend/augment rendered content (inside content)

While working on snapguidist I had some idea on that. Now that I've closed a very important deadline at work I can finally work on it. Next days I will share more about this with you!

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 1, 2017

@cef62 Ping ;-) Do you have some time now?

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 4, 2017

I've started digging through the new 5.x implementation. I really like the new AST based parsing, great job @sapegin!
At the moment the part bugging me the most is that there's no effective way to assign a unique ID to every parsed snippet. This is something very important to support a plugin API in my opinion, in snapguidist for example we could use it to avoid unnecessary execution of unchanged snippets.

At the moment the most viable solution I can think of, but I'm not fond of it, is to support a sort of front matter where the user can expose data to the plugins and configure them.

What I think could be a interesting way to expose a plugin API is to wrap the RSG component, we want expose as customizable parts, with an HoC connected to styleguidist configuration and access the registered plugin. Every plugin register itself and the final user control the registering order. That approach would allow a single plugin to customize any required aspect of the UI.

I want to share a more complete idea as soon as possible. Next two weeks I'll be participating to Jsconfit and Codemotion Amsterdam, then I'll be finally able to give my full attention to the plugin API and snapguidist.

Meanwhile what do you think of the idea of using HoCs to connect plugins to styleguidist? And, do you have any better idea on how to assign a unique ID to every snippet of code?

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 4, 2017

I think unique ID generation should be part of the core but I don’t know how to do expect use just indexes like Jest does itself.

About HOCs: sounds like a good idea, do you know how we could easily implement that? I also think that we should start from predefined extension points (toolbars and tabs) like I describe in #426 — it should be easier to implement and would be enough for many plugins, for Snapguidist as well if I’m not missing something ;-)

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 4, 2017

I think unique ID generation should be part of the core but I don’t know how to do expect use just indexes like Jest does itself

Jest use the assertion title as unique ID but we don't have it. This is why I'm thinking of a front matter approach. That approach would also open up snippet-level configuration of plugins.

About HOCs: sounds like a good idea, do you know how we could easily implement that?

I've a couple of ideas and it's what I hope to share in more detail asap.

I also think that we should start from predefined extension points (toolbars and tabs) like I describe in #426

I definitely agree with you!

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 4, 2017

Front matter is a nice idea but I’m afraid we can’t require users to write it for each example just to be able to use Snapguidist. We could use the text in a paragraph in front of the example. It’s not perfect but probably better than indexes.

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 4, 2017

but I’m afraid we can’t require users to write it for each example

Yep, this is my exact feeling about it. But could/should be optional. I mean the plugin author could support an explicit title/ID/whatever to support specific features, if the user doen't care about those features (like test execution optimization) can simply skip the front matter.

We could use the text in a paragraph in front of the example.

That is a nice idea to avoid unnecessary repetitions on every snippet.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 4, 2017

So, yeah, we could generate these IDs in the core and pass them to your plugin’s tab component or whatever ;-)

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 4, 2017

So, yeah, we could generate these IDs in the core and pass them to your plugin’s tab component or whatever ;-)

Yes and no. We can just recognize snippet positional index and their content. If the user change the snippet order and the content we don't have any way to be sure of which snippet whas assigned to which ID.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 4, 2017

If we use text than most likely the text will be moved with the snippet and we don’t have a better idea anyway ;-)

@MicheleBertoli

This comment has been minimized.

Copy link
Member

MicheleBertoli commented May 4, 2017

We can't assume there's always going to be a text above the snippet, right?

Jest is using a combination of name + index, and we are doing the same at the moment.
I can't see why we can't keep on doing it.

Afaik we don't have evidence of any performance issue, therefore I wouldn't worry about it.

Thanks @sapegin @cef62.

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 4, 2017

We can't assume there's always going to be a text above the snippet, right?

No we can't

Afaik we don't have evidence of any performance issue, therefore I wouldn't worry about it.

I was thinking not only about snapguidist but also to possibly more heavy integration that could benefit from more optimization.

Anyway I agree with you that is not fundamental, but a nice addition to have anyway.

@sapegin sapegin referenced this issue May 10, 2017

Merged

Feat: Component Metadata #435

3 of 6 tasks complete
@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 18, 2017

To start working on the API something very simple: adding buttons to component toolbar. What do you like more 1 or 2?

// styleguide.config.js
module.exports = {
	// ...
	plugins: [
		require('styleguidist-plugin-foo')({ some: 'options' })
	]
}
// Plugin that adds a button to each component toolbar
import React from 'react';
import ToolbarButton from 'react-styleguidist/lib/rsg-components/ToolbarButton');
import MdScreenShare from 'react-icons/lib/md/screen-share');
export default /* plugin options */ options => /* style guide config */ config => {
	return {
		// Buttons that should be added to a component toolbar
		// 1.
		getComponentToolbarButtons({ name, urlAnchor, urlIsolated, urlNoChrome }) {
			return [
				<ToolbarButton to={`http://sizzy.co/?url=${encodeURIComponent(urlNoChrome)}`} title="Open in Sizzy" blank><MdScreenShare /></ToolbarButton>
			];
		},
		// 2.
		toolbarButtons: [
			({ name, urlAnchor, urlIsolated, urlNoChrome }) =>
				<ToolbarButton to={`http://sizzy.co/?url=${encodeURIComponent(urlNoChrome)}`} title="Open in Sizzy" blank><MdScreenShare /></ToolbarButton>
		];
	};
};
@okonet

This comment has been minimized.

Copy link
Member

okonet commented May 19, 2017

IMO always go with functions since it's much more flexible in a long-term.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 19, 2017

@okonet Both are functions. The difference is only in an array placement: inside the function or outside. I like the second one more because it‘s just a component.

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 19, 2017

I also like the 2nd variant.
If I understand correctly any plugin it will produce an object containing several configuration hooks (toolbarButtons is the only one at the moment). react-styleguidist will chain all configurations from all registered plugins in the given order. Is that right? I like the approach!

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 19, 2017

@cef62 Yeah, that’s the idea for simple things like toolbar buttons. And after that we can think what else we need.

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 19, 2017

I agree! Do you have a working branch or is just an idea? I'd love to help on this ;)
I'm finally done with conferences for a little while and I'll finally be able to contribute to snapguidist and styleguidist.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 19, 2017

@cef62 There’s the ui branch, right now I’m working on a component / section toolbar. I hope I could finish it on the weekend and commit. So far there’s not much to see ;-)

@cef62

This comment has been minimized.

Copy link
Collaborator

cef62 commented May 19, 2017

Ok thanks! I'll look at it and I'll see how I can contribute to move toward the API you proposed!

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented May 19, 2017

@cef62 I think I need to implement the first version of the API with only component toolbar extension and then you could help with things we’ll need for Snapguidist.

@sapegin sapegin changed the title [discussion] Extensions API Extensions API May 23, 2017

sapegin added a commit that referenced this issue Jun 8, 2017

Changelog: 🚀
## New features

### UI refresh

Welcome our refined UI! More consistent and clean:

![](https://d3vv6lp55qjaqc.cloudfront.net/items/3j1P0K1a1Q451t2Z2p3G/Screen%20Recording%202017-06-08%20at%2008.54%20PM.gif)

🦊 This is the first part of the planned improvements and a base for upcoming plugin API, see #426 and #354 for more details — we need your feedback there 🚀

🍕 Thanks to @SaraVieira and @n1313 for help! ❤️

### Props & methods are hidden by default

Use the new config option [showUsage](https://react-styleguidist.js.org/docs/configuration.html#showusage) to restore the old behavior.

## Bug fixes

### Crash when using a defaultProp that is not listed in props (#437 by @ankri)

### Isolated examples inside sections

### Issues with `position: relative` (#441)

### Ugly isolated example link (#235)

sapegin added a commit that referenced this issue Jun 8, 2017

Changelog: 🚀
## New features

### UI refresh

Welcome our refined UI! More consistent and clean:

![](https://d3vv6lp55qjaqc.cloudfront.net/items/3j1P0K1a1Q451t2Z2p3G/Screen%20Recording%202017-06-08%20at%2008.54%20PM.gif)

🦊 This is the first part of the planned improvements and a base for upcoming plugin API, see #426 and #354 for more details — we need your feedback there 🚀

🍕 Thanks to @SaraVieira and @n1313 for help! ❤️

### Props & methods are hidden by default

Use the new config option [showUsage](https://react-styleguidist.js.org/docs/configuration.html#showusage) to restore the old behavior.

## Bug fixes

### Crash when using a defaultProp that is not listed in props (#437 by @ankri)

### Isolated examples inside sections

### Issues with `position: relative` (#441)

### Ugly isolated example link (#235)
@stepancar

This comment has been minimized.

Copy link
Member

stepancar commented Jun 9, 2017

i like 1st variant

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Jun 9, 2017

@stepancar Any particular reason why?

@stepancar

This comment has been minimized.

Copy link
Member

stepancar commented Jun 9, 2017

@sapegin, because in function we can implement any logic to select which buttons should be plased to example. For example, we want to add theme switcher button for several components which support theme switching using context.

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Jun 9, 2017

@stepancar In the second you could return null ;-) Maybe you have some other ideas?

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Jun 10, 2017

What if we make it more explicit like this?

// Plugin that adds a button to each component toolbar
import React from 'react';
import ToolbarButton from 'react-styleguidist/lib/rsg-components/ToolbarButton');
import MdScreenShare from 'react-icons/lib/md/screen-share');
export default /* plugin options */ options => /* style guide config */ config => {
	return {
		// Similar: onSection, onExample, etc.
		onComponent({ addToolbarButton }, { name, urlAnchor, urlIsolated, urlNoChrome }) {
		   addToolbarButton(
			<ToolbarButton to={`http://sizzy.co/?url=${encodeURIComponent(urlNoChrome)}`} title="Open in Sizzy" blank><MdScreenShare /></ToolbarButton>
			);
		},
	};
};
@okonet

This comment has been minimized.

Copy link
Member

okonet commented Jun 10, 2017

@sapegin I like this ↑

How will it be defined there the extension is happening? Is this pre-defiend by RSG?

@sapegin

This comment has been minimized.

Copy link
Member

sapegin commented Jun 22, 2017

I’ve created a PR with some experiments: #505.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment