alphagov / govspeak

Markdown extension library for Government editors
http://govspeak-preview.herokuapp.com/
MIT License
85 stars 22 forks source link

HTML line-break elements not appearing in "contact" containers #55

Open floehopper opened 9 years ago

floehopper commented 9 years ago

According to the relevant section of the README, lines demarcated with $C are supposed to be wrapped in a div with class contact and each line is supposed to be separated by a line-break elements <br />. The wrapping is working as described, but it appears that no line-break elements are being added.

I think this is why the formatting of the Tax Credit Helpline contact details on pages like this one are not formatted as I would expect based on the source govspeak markup

Does anyone know whether this govspeak behaviour is intentional (and the docs are out-of-date) or is the behaviour incorrect?

Note that there looks as if there's specific code in place to add line-breaks for address containers (demarcated by $A).

floehopper commented 9 years ago

I can confirm that the Smart Answers app has been working around this issue by adding two trailing spaces to lines within a contact box. This makes use of Kramdown's line-break mechanism.

As explained in this commit note, it's all too easy for these trailing spaces to get accidentally removed e.g. by a developer's editor automatically stripping trailing whitespace. A slight improvement on this is to use the alternative Kramdown line-break mechanism which involves adding two trailing backslashes - hopefully it should be more obvious that these have been added intentionally.

However, both these solutions feel like workarounds for incorrect behaviour in Govspeak. I'd like to submit a PR to fix the behaviour and bring it in line with the documentation, but before I put the time in, I'd like to be confident that the changes will be accepted. I'm conscious that Govspeak is used by a number of other applications and I wouldn't want to break them.

Does anyone have any thoughts? How do other apps, e.g. Whitehall, deal with this?

alext commented 9 years ago

I think that would be a good change to make, however it would be important that this is done in a backwards-compatible manner for markup that already has the manually inserted line-breaks (ie markup with these manual line-breaks shouldn't get an extra line-break as a result of this change.)

floehopper commented 9 years ago

@alext: Thanks for your reply. I think that a solution like that might make the code quite complex/brittle. An alternative solution might be to make the new behaviour the default in a new version of the gem, but provide an option to retain the old behaviour. Another benefit of this latter approach would be that any new apps which start using Govspeak would rely on the default/correct behaviour. What do you think?

alext commented 9 years ago

I see your point, this will add more complexity to the code.

I'm just aware of the volume of content out there that will be written for the existing behaviour. We don't want to end up in a place where we can't upgrade an app to a newer version of govspeak for fear of breaking existing content.

TBH, both approaches have their issues. I'm not sure which option is best...