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

Tables: use style=text-align for table alignment #357

Merged
merged 1 commit into from Jun 16, 2020
Merged

Tables: use style=text-align for table alignment #357

merged 1 commit into from Jun 16, 2020

Conversation

codebykat
Copy link
Contributor

Table alignment currently uses align=left|center|right which is a deprecated attribute with limited support in modern browsers.

At the moment I'm having to post-process the Markdown generated by markdown2.py in order to replace the aligns with text-align as we need good cross-browser support (notably, Firefox completely ignores the align attribute).

See:

This PR switches instead to the recommended and more widely-supported style=text-align: https://caniuse.com/#feat=mdn-css_properties_text-align

Additionally, in updating the tests I found that they contained a spelling error and that they have their lefts and rights switched (I double-checked the expected output for the syntax on https://help.github.com/en/github/writing-on-github/organizing-information-with-tables ).

I know the extras tests are imported from an upstream repo, so I'm not sure how you would prefer to handle this inconsistency, but I thought it was sufficiently confusing to have it read "left" when it expected "right" that it was worth updating.

@nicholasserra
Copy link
Collaborator

Thank you! This looks great. Appreciate updating the extras test. Seems the tables test actually passes when ran, so i'm gonna merge all of this and then fix the test setup to run the extras table test in CI.

@nicholasserra nicholasserra merged commit c566064 into trentm:master Jun 16, 2020
@codebykat codebykat deleted the update/use-text-align-for-table-alignment branch June 16, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants