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

Add documentation for component conventions and principles #14

Merged
merged 8 commits into from Aug 7, 2017
Merged

Add documentation for component conventions and principles #14

merged 8 commits into from Aug 7, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Aug 3, 2017

  • Create two pages for conventions and principles
  • Include screenshot
  • Reduce size of initial README and link out to component details
* Output from GOV.UK Publishing Frontend team workshop
README.md Outdated
A lead-paragraph component would be included in a template like this:

```erb
<%= render 'components/lead-paragraph', { text: "A description is one or two leading sentences." } %>

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

Are the curly braces needed?

This comment has been minimized.

@nickcolley

nickcolley Aug 7, 2017
Contributor

Might be worth always having them, since the guide also does this.

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Removed.

| -- | -- |
| `.app-c-` | Component lives within the frontend application |
| `.pub-c-` | Component is shared between publishing frontends and lives in static |
| `.govuk-c-` | Component originates from [GOV.UK Frontend](https://github.com/alphagov/govuk-frontend) |

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

This whole opening section relates to styles, so I'd start with the 'Structure' section and move this into the 'Styles' section. Then the link to the GOV.UK frontend doc with their BEM convention is more usefully placed (https://github.com/alphagov/govuk-frontend/blob/master/docs/coding-standards/css.md)

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

I discussed this with @nickcolley. I've put it first as I think it's the most important thing. It mirrors he GOV.UK Frontend docs and structure follows closely behind.

This comment has been minimized.

@nickcolley

nickcolley Aug 7, 2017
Contributor

I'm on the fence with this one, I think it's the most important for the GOV.UK Frontend project but not for us?

I thought the same as Andy, I was unsure why I was seeing classes and what they related to. Maybe it'd make more sense with the context of the structure first.

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

I think the confusing thing is that styles are discussed twice. I think all the style stuff should be together.

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

I could add the heading "Namespacing"?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Added the Namespacing heading. I think that clarifies things a little.


| Property | Required | Description |
| -- | -- | -- |
| filename | Required | Filename of `.yml` file must match component partial name |

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

Would an example of a filename be helpful?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Added in paragraph above.

The link must:
* be keyboard focuasable

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

focuasable

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Fixed.

fixtures:
default:
some_parameter: 'The parameter value'
```

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

Fixtures could perhaps use some more explaining. Maybe link to an example?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

* defines a convention for components to follow
* lints components for consistent coding style
* makes it easy to build, move or delete components
* makes it easy to arrange or compose components without glue code

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

Worth a brief definition of 'glue code'?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Updated wording.

* [follows convention](component_conventions.md)
* is isolated
* is tested
* can be translated

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

We've not talked about this much since starting this component work. Is there something we can link to here to give a bit more detail about how translations should be handled in applications?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Not yet. You're right that it needs expansion. Maybe add an issue?

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

Added.

* makes it easy to build, move or delete components
* makes it easy to arrange or compose components without glue code
* makes components discoverable
* hides the internal implementation of a component from applications

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

I wonder if this needs further definition - I know I benefitted greatly once the understanding of why passing a class into a component for it to use was a bad idea.

This comment has been minimized.

@nickcolley

nickcolley Aug 7, 2017
Contributor

Seems like this and the 'glue code' example could benefit from the real world examples we've come across?

This comment has been minimized.


## Write components in your application

Components are packages of template, style, behaviour and documentation that live in your application.

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

This is nit-picking, I know, but can we really describe a set of files in different directories as a package? Perhaps an 'associated group of files'?

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Package is fine I think, and terser than 'associated group of files'. If the component files aren't grouped well enough then the problem isn't a naming one.

README.md Outdated
* [meet the component principles](docs/component_principles.md)
* [follow component conventions](docs/component_conventions.md)

A lead-paragraph component would be included in a template like this:

This comment has been minimized.

@andysellick

andysellick Aug 7, 2017
Contributor

No need for hyphen in 'lead paragraph'.

This comment has been minimized.

@fofr

fofr Aug 7, 2017
Author Contributor

Fixed 🎩

fofr added 2 commits Aug 3, 2017
* Link to GOV.UK frontend where appropriate
* Explain namespacing
* Clarify component structure and documentation
@fofr fofr force-pushed the docs branch from dad4633 to d55359c Aug 7, 2017
fofr added 5 commits Aug 3, 2017
* Move component details to the top, but link out to the important
parts for reference
* Group set up and configuration under “Set up a component guide”
@fofr fofr force-pushed the docs branch from 6850536 to a4467c9 Aug 7, 2017
@fofr fofr merged commit be49cf1 into master Aug 7, 2017
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@fofr fofr deleted the docs branch Aug 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.