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

Allow table cells to have a text alignment #71

Merged
merged 1 commit into from May 3, 2016
Merged

Allow table cells to have a text alignment #71

merged 1 commit into from May 3, 2016

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 29, 2016

Kramdown has a markdown pattern for allowing left, centred and right aligned cell contents:
http://kramdown.gettalong.org/quickref.html#tables

To achieve this it uses a style property, eg style=‘text-align: right’
https://github.com/gettalong/kramdown/blob/60795eb95f1588d41e105407add2d3ba6a7fc101/lib/kramdown/parser/html.rb#L446-L449

We don’t want to allow any styles to be applied, just the subset that Kramdown uses. Instead, whitelist the attribute for table cells, but use a Sanitise transformer to remove the style unless it matches the 3 possible values it could have.

Part of:
https://trello.com/c/6ld11Eck/346-right-alignment-in-tables-in-govspeak-medium

Markdown example (left aligned, centre aligned, right aligned):

| Header1 | Header2 | Header3 |
|:--------|:-------:|--------:|
| cell1   | cell2   | cell3   |
| cell4   | cell5   | cell6   |
Header1 Header2 Header3
cell1 cell2 cell3
cell4 cell5 cell6
Kramdown has a markdown pattern for allowing left, centred and right
aligned cell contents:
http://kramdown.gettalong.org/quickref.html#tables

To achieve this it uses a style property, eg `style=‘text-align: right’`

We don’t want to allow any styles to be applied, just the subset that
Kramdown uses. Instead, whitelist the attribute but use a Sanitise
transformer to remove the style unless it matches the 3 possible values
it can have.
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 29, 2016

An alternative to letting these styles through might be to transform Kramdown's output to use an align attribute. Github's rendering uses align. We'd also need to add the styles to correctly align the content.

@boffbowsh boffbowsh merged commit d4d4b70 into master May 3, 2016
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
@boffbowsh boffbowsh deleted the allow-align branch May 3, 2016
@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented May 3, 2016

An alternative to letting these styles through might be to transform Kramdown's output to use an align attribute. Github's rendering uses align. We'd also need to add the styles to correctly align the content.

I don't think the align attribute would need styling (demo), but it's been dropped in HTML 5.

I'm not a big fan of the inline styles, we could use align regardless, or alignment classes?

@fofr
Copy link
Contributor Author

@fofr fofr commented May 3, 2016

We should raise an issue upstream on Kramdown.

@fofr fofr mentioned this pull request May 3, 2016
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.