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 statistic headline markdown #60

Merged
merged 7 commits into from Oct 9, 2015
Merged

Add statistic headline markdown #60

merged 7 commits into from Oct 9, 2015

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Oct 7, 2015

https://trello.com/c/z4YmHWDp/127-html-pubs-add-markdown-to-allow-users-to-add-large-font-and-graphs-medium

Departments are about to create and publish their Single Departmental Plans. These will be published to GOV.UK as HTML publications. Departments need to be able to pull out important figures periodically throughout the HTML publication in large font.

  • Add the {stat-headline} markdown pattern
  • Add <aside> as an acceptable HTML element

See also: https://github.com/alphagov/whitehall/compare/stat-headlines

{stat-headline}*13.8bn* years since the big bang{/stat-headline}

screen shot 2015-10-08 at 17 01 17

cc @alextea

fofr added 5 commits Oct 5, 2015
The highlight box isn’t pink.
Allow users to add big numbers with a smaller label.

These use an `<aside>` tag as they are secondary content.
Prevent the sanitizer from raising a problem when using the new markdown
When showing multiple stats side by side, they need to be wrapped for
correct styling.
@jamiecobbett
Copy link
Contributor

@jamiecobbett jamiecobbett commented Oct 7, 2015

Apart from a minor comment, this looks 👍 to me.

cc @dsingleton and @heathd as a person who has worked a lot on Govspeak/Whitehall.

README.md Outdated

Used in HTML publications.

Statistic headlines highlight important numbers in content. Displays a statistic as a large number with a description. The statistic and description must make sense when read aloud. The giant number is denoted with `**`.

This comment has been minimized.

@jamiecobbett

jamiecobbett Oct 7, 2015
Contributor

The giant number is denoted with **.

I can see that is consistent with the Answer section, but why is it not something like "wrapped with *'s"?

@alextea
Copy link

@alextea alextea commented Oct 7, 2015

Interested to hear people's thoughts on the govspeak syntax. We have many different styles, and I think it would be good to agree a consensus for any new patterns (and then maybe port existing patterns to new style, and deprecate the old styles).

For the record, I think hyphen delimited text reads better than camel case. Do we need the :: parts?

@heathd
Copy link
Contributor

@heathd heathd commented Oct 8, 2015

looks good to me, it's using (one of) the existing patterns.

I agree with @alextea that there's inconsistency, but this isn't making it any worse, cleaning up the inconsistencies will be a bigger piece of work.

@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Oct 8, 2015

The markup and changes to govspeak itself look good to me 👍

Are you planning on adding the CSS changes in WH to the the Govspeak component on static? It would be good to see a fixture for the Component Guide, to explicitly show how the state-headline will render in that situation?

Groups aren’t necessary _yet_.

In user research the group has been used in a way that causes content
to render poorly. Content of different heights would render incorrectly
if more than 2 stats were used in a group. Content with long numbers
would overlap each other at thinner page widths when using standard
grid markup.

While styles could be modified to avoid these bugs, the simplest
approach for now is to remove this capability altogether. It may need
to be added if a user need arises as departments work on their single
departmental plans.
@fofr
Copy link
Contributor Author

@fofr fofr commented Oct 8, 2015

Looking at the {::stat-headline} pattern with a few users, it's quickly apparent that it's very easy to make a couple of mistakes:

  • {:: can be confused with {:
  • {:/ can be confused with {/: or {::/

This type of error is then very hard to spot and fix.

@fofr
Copy link
Contributor Author

@fofr fofr commented Oct 8, 2015

I've removed {::stat-headline-group} in de962cd

Patterns using `{::` and `{:/` are confusing and lead to errors that
are difficult to spot.
@fofr fofr changed the title [Do not merge] Add statistic headline markdown Add statistic headline markdown Oct 8, 2015
@fofr
Copy link
Contributor Author

@fofr fofr commented Oct 8, 2015

I've updated the markdown from {::stat-headline} to {stat-headline} to reduce user error. This adds a new pattern into govspeak but is preferable to the existing one. The previous {:: } pattern is not widely known to Whitehall users.

If people are happy with this approach and the new pattern this PR can be merged.

@jamiecobbett
Copy link
Contributor

@jamiecobbett jamiecobbett commented Oct 9, 2015

👍

1 similar comment
@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Oct 9, 2015

👍

dsingleton added a commit that referenced this pull request Oct 9, 2015
Add statistic headline markdown
@dsingleton dsingleton merged commit 42c79fc into master Oct 9, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@dsingleton dsingleton deleted the big-numbers branch Oct 9, 2015
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

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