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

Enable isolation of a single fenced block example #225

Merged
merged 5 commits into from
Nov 14, 2016

Conversation

cef62
Copy link
Collaborator

@cef62 cef62 commented Nov 14, 2016

Hi @sapegin
first of all thanks for this project! I really love it and we use it in several projects.
In many of our library projects is the only available compiled output and we often need to isolate a single example from a readme file.

This PR add the ability to isolate a single code example to simplify the development process.
Would you like to add this feature to the official repo?

I'd love to have your feedback!
Cheers

@cef62 cef62 changed the title Feat: Enable isolation of a single fenced block example Enable isolation of a single fenced block example Nov 14, 2016
@sapegin
Copy link
Member

sapegin commented Nov 14, 2016

Hi @cef62! That looks very interesting, thank you very much for the PR!

I’ll review the code a bit later.

Using indexes looks fragile (what if you add or move an example?) but I have no better idea how to implement it.

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

Thanks @sapegin, let me knows you want me to change something!

Using indexes looks fragile (what if you add or move an example?) but I have no better idea how to implement it.

I agree with you but I didn't want to touch too much your current code structure and all texts and fenced blocks are stored in a single array with just numeric indexes. Anyway I don't think should be a real problem because the index will change only if you add other texts or fenced blocks to the readme file and that shouldn't happen while you are debugging a single example.
Probably the only problematic scenario is if you bookmark a single example and then the readme file changes.

I think I can get a more robust solution but that would involve the user somehow to explicitly always specify an ID for the fenced block; personally, as user, I'd prefer avoiding it but it's a viable option.

@sapegin
Copy link
Member

sapegin commented Nov 14, 2016

I think indexes are fine unless someone find better solution that doesn’t affect developer experience.

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do something with the UI. Right now it’s inconsistent and a bit too loud for such a feature. (But I we can merge it and think more later.)

Otherwise it’s great ;-)

if (targetComponentName) {
components = [
...filterComponentsByExactName(components, targetComponentName),
...filterComponentsInSectionsByExactName(sections, targetComponentName),
];
sections = [];
sidebar = false;

if (components.length === 1 && _.isFinite(targetComponentIndex)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use cherry-picking for Lodash for consistency: import isFinite from 'lodash/isFinite'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we need some comments here to explain all the things we’re doing: isolated component, isolated example, etc.

@@ -14,7 +14,7 @@
composes: font from "../../styles/common.css";
composes: border link from "../../styles/colors.css";
position: absolute;
right: -1px;
right: 138px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not cool ;-)

@@ -43,6 +50,7 @@ function renderStyleguide() {
components={components}
sections={sections}
sidebar={sidebar}
singleExample={singleExample}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks it would be better to use context here, like for config. Probably make a new object in config for sidebar and singleExample.

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

We need to do something with the UI. Right now it’s inconsistent and a bit too loud for such a feature (But I we can merge it and think more later.)

Yep the UI is not that good... my intention was to understand if you'd like the feature before thinking on the look & feel.

Otherwise it’s great ;-)
Thanks :)

I've addressed your comments, let me know if you want further tweaks.

@sapegin
Copy link
Member

sapegin commented Nov 14, 2016

Thanks! Do you have any ideas on how to improve the UI? If not I’d merge it and try to do something myself before releasing a new version.

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

I was thinking to move the link to isolate a single example to the upper-right of the rendered fenced block. Something similar the link you already have in place in the upper-right corner of every rendered component.

What do you think? Would you like I add it to the PR?

@sapegin
Copy link
Member

sapegin commented Nov 14, 2016

I was thinking about that too. And show it on hover. At least it would be more consistent with component isolation UI. So please add it to the PR and I’ll merge it.

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

Great! I'll add it to the PR.

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

I've made the discussed changes, let me know if it's ok for you!

@sapegin sapegin merged commit c26ec66 into styleguidist:master Nov 14, 2016
@sapegin
Copy link
Member

sapegin commented Nov 14, 2016

Cool, that’s much better!

@cef62
Copy link
Collaborator Author

cef62 commented Nov 14, 2016

Thanks! I'm happy you liked the addition 👍

sapegin added a commit that referenced this pull request Nov 14, 2016
## New features

Now you can open single example (not a whole component) in isolated mode:

![](http://wow.sapegin.me/2037140t1y0O/Image%202016-11-14%20at%206.19.19%20PM.png)

(#225 by @cef62)
sapegin added a commit that referenced this pull request Nov 15, 2016
## New features

### Isolated mode for examples

Now you can open single example (not a whole component) in isolated mode:

![](http://wow.sapegin.me/2037140t1y0O/Image%202016-11-14%20at%206.19.19%20PM.png)

(#225 by @cef62)

### Config option to debounce preview rendering while live editing

Default value is 500. That means you’ll not see that annoying red error message box while you’re typing.

(#227 by @cef62)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants