Skip to content
This repository has been archived by the owner on Oct 1, 2019. It is now read-only.

'Secondary' demos #184

Closed
dan-searle opened this issue Apr 28, 2014 · 16 comments
Closed

'Secondary' demos #184

dan-searle opened this issue Apr 28, 2014 · 16 comments

Comments

@dan-searle
Copy link
Contributor

Currently every demo listed in the origami.json will be shown expanded in the Registry.

For some modules, there are far more possible style combinations than would be desirable to show on the module's registry page, but would be very useful to a developer to ensure nothing has been broken in less common usage scenarios. o-table would be a good example of this.

@kavanagh and I propose that we have a means of listing such 'secondary' demos in the registry, but in a less prominent way, perhaps just as a collapsible list of links which would open in a new window.

Maybe the simplest way of specifying these would be to introduce a second property in origami.json which would be an array of these pages:

    "demos": ["demos/demo1.html"],
    "secondaryDemos": ["demos/secondary.1.html"]

We could do something similar in the config.json used by origami-build-tools to build the demos:

    "demos": {
        "demo1": {
            "template": "demos/src/demo1.mustache"
        }
    },
    "secondaryDemos": {
        "secondary1: {
            "template": "demos/src/secondary1.mustache"
        }
    }

I don't think 'secondary' is the best choice of word though.

@kavanagh
Copy link
Member

Did you mean to have a list?

{
    "demos": [
        {
            "template": "demos/src/demo1.mustache",
            "name": "Simple Demo"
        }
    ],
    "secondaryDemos": [
        {
            "template": "demos/src/secondary1.mustache",
             "name": "Advanced Demo with loads of frills"
        }
    ]
}

Making it as an array ensures it's ordinal and the developer is able to define the order they are show on the registry website.

@kavanagh
Copy link
Member

Would it be cleaner to have

{
    "demos": [
        {
            "template": "demos/src/demo1.mustache",
            "name": "Demo 1"
        },
        {
            "template": "demos/src/secondary1.mustache",
             "name": "Advanced Demo 1",
             "secondary": true
        }
    ]
}

@kavanagh
Copy link
Member

It would also be useful to have an optional description for each demo.

For some of these advanced/secondary demos it might not be immediately obvious why the demo exists, and what use cases the creator was intending to test/show. The description could be displayed next to the list of links (on the registry site) Dan mentioned.

@dan-searle
Copy link
Contributor Author

I agree an array of objects would be better.

Rather than trying to pick a word to describe the 'secondary/advanced' designation of particular demos, perhaps the property should just indicate how it should be displayed in the registry:

{
    "demos": [
        {
            "template": "demos/src/demo1.mustache",
            "name": "Demo 1"
        },
        {
            "template": "demos/src/secondary1.mustache",
             "name": "Advanced Demo 1",
             "expanded": false
        }
    ]
}

@kavanagh
Copy link
Member

kavanagh commented May 1, 2014

In the meeting discussed:

  • name is optional. uses currently method of replace dashes to spaces etc
  • adding an optional description to communicate what is be demoed/tested
  • screenshots/wraith/diffing etc. @wheresrhys to make a seperate issue
  • @dansearle-ft to raise issues on registry for improvements to cover this.
  • registry website could be easier to navigate the long page (with jump-links/tabs at the top)

@triblondon
Copy link
Contributor

This all sounds OK to me. Can we made the object optional so that it's backwards compatible? This would then be valid:

{
    "demos": [
        "demos/simple.html",
        {
            "name": "A complex demo",
            "path": "demos/complex.html",
            "expanded": false,
            "description": "Shows some edge case that isn't important to display in the demo on the registry"
        }
    ]
}

@triblondon
Copy link
Contributor

I don't like 'template' as a key, by the way. Remember that demos are not templates - they cannot require a build step.

@dan-searle
Copy link
Contributor Author

Yes, I think we've confused the config.json used by origami-build-tools to generate demos (where it is a template that's being referred to), and the demos property in the origami.json (where it's a plain HTML file).

The config.json is only required if you're using OBT to build the demos, whereas the entries in the origami.json are always required.

Currently there's some magic going on when you use OBT to build demos:

  • In --local mode, it will delete the demos entry in origami.json
  • In --buildservice mode, it will re-populate the demos entry in origami.json based on the demos successfully created from the config.json

This is to try to keep the origami.json up to date, and avoid local demos showing up in the Registry, but it's not at all clear. This could be resolved with OBT issue 17.

I think we should perhaps remove this magic from OBT, and instead perhaps have a pre-commit verify OBT command that would check the contents of the demos property of origami.json matches the contents of the demos/ folder.

@wheresrhys
Copy link
Contributor

I think we should perhaps remove this magic from OBT

I really like that magic. I'd prefer to keep it in but behind a y/n prompt in the cli before the demo task completes

@wheresrhys
Copy link
Contributor

and could also have a `Remove built js and css files? y/n' prompt to make sure they don't get committed. Magic to tidy up bower.json's ignore list could also be put behind a prompt

@kavanagh
Copy link
Member

kavanagh commented May 6, 2014

In --local mode, it will delete the demos entry in origami.json

Why do this? Should it just leave the origami.json alone?

In --buildservice mode, it will re-populate the demos entry in origami.json based on the demos successfully created from the config.json

I'd put this magic behind a CLI flag. i.e --save-config or --update-config

@dan-searle
Copy link
Contributor Author

Why do this? Should it just leave the origami.json alone?

It's so if you commit the HTML files that have been built for local, they won't show up in the registry.

I think some of these problems could be avoided or mitigated if we were to address this OBT issue.

@triblondon
Copy link
Contributor

Can we bring this back to the secondary demos issue? Are we happy with the proposal in #184 (comment)?

@dan-searle
Copy link
Contributor Author

I'm happy with that, but OBT ought to be changed around the same time the registry is changed to support this, otherwise it will wipe out the hand-coded demos entry in the origami.json.

@triblondon
Copy link
Contributor

OK, I'll implement in the registry.

@triblondon
Copy link
Contributor

OK, this is implemented:

http://registry.origami.ft.com/components/o-grid@2.1.4

I've opened Financial-Times/origami-build-tools#22 to update the build tool so that it doesn't overwrite granular demo data with a simple array list.

@triblondon triblondon removed their assignment May 14, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants