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

Make page per section configuration less confusing #1145

Open
sapegin opened this issue Sep 20, 2018 · 20 comments

Comments

@sapegin
Copy link
Member

commented Sep 20, 2018

We have quite a lot of issues related to sections configuration, especially the new sectionDepth option. It seems hard to understand (and honestly I don't understand it too).

@okonet @bebraw @rafaesc How can we improve this API? It would probably make more sense to have an option on each section that will define whether this section should be rendered with all subsections or not. But I don't have enough experience with pagePerSection option to say much.

Related issues: #1139 #1137 #1058

@okonet

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

My suggestion still stands: #892 (comment)

Although I'd probably change the name to something more clear. Ideas?

@bebraw

This comment has been minimized.

Copy link

commented Sep 20, 2018

@okonet's approach looks good. The nice thing is that although it's verbose, the users can program a lot of that away in their config. That's what I've ended up doing myself.

I wonder if recursive definition would make sense. Then you could do sections inside sections.

@sapegin

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

Yeah, I like it too! I think codeSamples and propsMethods are already working this way, so we should make separatePage work the same way.

@okonet

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

After thinking about it a bit more I think the root problem is with naming of sections. Also the fact that components property "generates" sections per component makes the whole more complex.

What if instead of bolting this on top of current structure we would introduce pages concept to styleguidist? Pages would become distinguished and uniq URLs automatically. Example:

module.exports = {
    // Each page gets its own URL
    pages: [{
        name: 'Documentation & Examples',
        slug: 'documentation', // optional
        // Sections are just anchors inside the page?
        sections: [
            {
                name: 'Design tokens',
                content: 'docs/tokens.md',
                codeSamples: 'hide',
                propsMethods: 'hide',
            },
            {
                name: 'UI Components',
                components: '...',
            },
            {
                name: 'UI Patterns',
                components: '...',
            }
        ]
    }, {
        name: 'About',
        content: './about.md'
    }]
}

Another thing I'm not sure in current design is also the difference between content and components which is very implicit (components will be added to content if both are provided).

To me the problem we're trying to solve is to make styleguidist into a web-site generator on steroids. Ultimately we could even use some other generator (like https://github.com/antwarjs/antwar) instead and only handle steroids part (components rendering, resoving etc.)?

Probably we should start with the ideal API and think this through?

@sapegin

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

I like the idea of starting from an ideal API, if we're going to make a breaking change anyway.

@okonet

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

@sapegin do you have something in mind? What about my proposal with pages and sections? One problem I can see already is that it can be tricky to mix pages and sections inside one page for example...

@sapegin

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

I'm not really sure what's the difference between page and section ;-/

@okonet

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

We can change the names if we feel they don’t fit but my idea was to distinguish between isolated pages and parts of one page (anchors)

@rafaesc

This comment has been minimized.

Copy link
Collaborator

commented Sep 21, 2018

I agree with separatePage approach, it will be easier to understand with a boolean flag.

Although I would like to standard the name of pages, sections, content and components.

@sapegin

This comment has been minimized.

Copy link
Member Author

commented Sep 21, 2018

@okonet So your idea is that the page is a separate route with a separate URL and pages can have sections inside and sections are headings with anchors inside a page? And we're replacing sections config option with pages? If my understanding is correct, then I like the idea ;-)

We'll need to fit external links somehow into this model.

@yakunins

This comment has been minimized.

Copy link

commented Sep 25, 2018

I guess nicely implemented href option will do job well. Like that:

  sections: [  
    {  
      name: 'Home',  
      content: './home.md',
      href: '/' // Back to component's root
    },
    {
      name: 'Visual',
      href: '#visual', // Just in-document navigation 
      components: ...
    },
    {
      name: 'Just a text, no action on click',
      href: false // No action on click
    }
  ]

So, href may be:

  1. true (default), to navigate over components and sections routes
  2. false, for taking no action :)
  3. extrenal, like http://...
  4. internal, like #section or #section/component
@sapegin

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2018

@yakunins What issues do you see with the current implementation of the href option?

@viljamis

This comment has been minimized.

Copy link

commented Oct 6, 2018

@sapegin This sounds like a good idea. Even though I’ve been now using sectionDepth option extensively (or I think so at least) on Vue Design System, I still don’t always quite understand what it is supposed to do as using f.ex. the components option always anyway adds another level of “depth” even when I don’t want it to.

@yakunins

This comment has been minimized.

Copy link

commented Oct 9, 2018

@sapegin These are not issues, looks more like features... Tested!

  • href="#section" (works fine)
  • href="http://google.com" (works fine)
  • href="http://google.com" external: true (works fine)
  • href=false (not giving a plain text, renders href="/#/name" instead)
  • href="" (not good, renders href="/#/name" instead)

Just to illustrate the idea:

export function LinkRenderer(_ref2) {
  ...
  return React.createElement(
    props.href ? "a" : "span",
    _extends...
  ...
@okonet

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

@yakunins I like the idea with href especially the fact it already exists and doesn't need a breaking change. Am I correct in thinking that it will cover all edge cases? Can you post an example config?

@yakunins

This comment has been minimized.

Copy link

commented Oct 9, 2018

Privet, @okonet!

That what I wish to have:

  ...
  pagePerSection: true,
  sections: [
    {
      name: 'All components',
      href: '/',
      content: './src/readme.md',
      pagePerSection: false
    },
    {
      name: 'Visual',
      href: false,
      sectionDepth: 2,
      components: () => ([
        path.resolve(__dirname, './src/visual/Text/', 'Text.tsx')
      ])
    },
    {
      name: 'Layout',
      href: false,
      sectionDepth: 2,
      components: () => ([
        path.resolve(__dirname, './src/layout/Grid/', 'Grid.tsx'),
        path.resolve(__dirname, './src/layout/Grid/', 'GridColumn.tsx'),
        path.resolve(__dirname, './src/layout/Grid/', 'GridRow.tsx')
      ])
    },
    ...

Some examples of plain text sections:

Collapsable sections headers:

@okonet

This comment has been minimized.

Copy link
Member

commented Oct 16, 2018

First of all, I don't like the presence of pagePerSection and sectionDepth. That's exactly the reason this issue exist.

Try answering the following question: What would be your preferred way of doing this if you could start from scratch?

To me, this still feels ambiguous: what exactly href: false is affecting? The page Visual? So how do I navigate there?

@Spenc84

This comment has been minimized.

Copy link

commented Oct 26, 2018

@okonet If I am understanding @yakunins correctly, href: false would mean that the section title 'Visual' would still appear in the sidebar with various components listed under it, but clicking on the word 'Visual' wouldn't do anything since 'Visual' itself doesn't have any content of it's own other than the Components listed under it which in this case seem to each have pages of their own.

I personally like Okonet's suggestion to clearly distinguish between pages and sections, where pages each have their own individual page and url, and sections simply become anchored section headers that exist on a page.

I also like Yakuni's use of the href property to customize what actually happens when you click on an item in the sidebar.

Something like this:

{
      name: 'Components', // is required
      href: '/Components',
      content: './readme.md',
      sections: [ ... ],
      pages: [ ... ],
      ...
}

They way I see it, you would use this same object template for both pages and sections and it would be interpreted in the following manner:

name | string : required |
Represents what is displayed in the sidebar and on the page or section header.

href | false, string, undefined |
If false, clicking on this item in the sidebar should do nothing and it should look visually different from other clickable links in the sidebar.
If undefined the href will be <parentURL>/${name} for pages or <parentURL>#${name} for sections.
If a string is specified then that string would be appended to it's parent's URL instead of the name attribute unless it begins with http:// or https:// in which case it would be treated as an external link.

content | string, undefined |
A string would represent the path to the .md file that should be used to populate the content of this page or section.
If content, sections, and href are all undefined then the sidebar link for this item should not be clickable, the same as if href were set to false.

sections | string, glob, object, array, undefined |
Sections should be displayed on a page below the specified content for that page (if any). If no content is defined then the content for the page should simply be a list of the specified sections. Each section should have a section header that matches the name of the Component or the name attribute of the section object. Each section header should have a page anchor that is determined by the href attribute (or the default case if the href attribute is undefined). Optionally (but ideally in my opinion) the contents of each section should be collapsible when the section header is clicked on. Perhaps another attribute could be included on a section object that indicates whether a particular section should start off in a collapsed state or not. |
When sections is a string it would represent the path to a single component which would be displayed as a section on it's parent's page.
When sections is a glob it would represent a glob pattern that identifies a list of components that should be displayed as sections on the parent's page.
When sections is an object it should take the same shape as the proposed format above and would be displayed as a single section on it's parent's page.
Alternatively sections could be an array of strings, globs, or objects that are compiled into a list of sections to be displayed on it's parent's page.

pages | string, glob, object, array, undefined |
The pages attribute would contain similar data to sections. Items listed under the pages attribute would still show up in the sidebar as nested items of the parent, but they would not be displayed on the parent's page but would be given a unique URL and page of their own instead.

@wwwebman

This comment has been minimized.

Copy link

commented Oct 31, 2018

Hi, guys! Here short example how we try to manage our sections:

{
    pagePerSection: true,
    sections: [
        {
            name: 'Components',
            sectionDepth: 1,
            description: 'Component',
            href: false, // want to disable this link.
            components: [
                resolve(__dirname, 'src', 'Button', 'Button.jsx'),
                resolve(__dirname, 'src', 'Input', 'Input.jsx'),
            ],
        },
    ],
}

Current state:
So, as you can see from config it will put all components on separate page. Also when we click 'Components' we will get empty page.

Desire state:

  1. href -> false
    We would like to be able to disable this link at all and be able to open nested components pages. So, for us @Spenc84 point makes sense:

If false, clicking on this item in the sidebar should do nothing and it should look visually different from other clickable links in the sidebar.

  1. href -> toggle feature
    Would be nice to have 'toggle' feature for some section links like in material-ui docs - https://material-ui.com/api/app-bar (click on 'Component API' section.)
@J-Kallunki

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

Hi! I have a problem using pagePerSection. If I want to have one section and components-function under that - it is not showing them per page. But if put another section under the first section is working properly (and adding sectionDepth: 2 that cannot be added for top level sections-array). So I end up having empty sub-sections with name-link and url-paths for nothing.

https://codesandbox.io/s/orlprkkk5?fontsize=14&moduleview=1

	pagePerSection: true,
	sections: [
		{
			name: 'Testing with section',
			content: './src/testing.md',
			components: () => {
				return glob.sync(path.resolve(__dirname, 'src/**/*.js'));
			},
			// Where do i put the sectionDepth for components?
		},
		{
			name: 'Testing with section again',
			content: './src/testing.md',
			sections: [
				{
					name: 'But needs an empty sub section',
					components: () => {
						return glob.sync(path.resolve(__dirname, 'src/**/*.js'));
					},
				},
			],
			sectionDepth: 2,
			// Oh here we can have it, but I don't need the sub-section
		},
	],
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.