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

Enforce right-alignment for cell in markdown tables #2534

Merged
merged 2 commits into from Aug 2, 2017

Conversation

@gnestor
Copy link
Contributor

@gnestor gnestor commented May 30, 2017

@takluyver
Copy link
Member

@takluyver takluyver commented Jun 20, 2017

I'm wary of adding !important rules because they're hard for anyone else to override. Are we so confident in our opinion of how tables should look that we're willing to more-or-less force it on everyone?

cc @ellisonbg

@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Jun 20, 2017

@takluyver I suppose not because in jupyterlab/jupyterlab#2151, @tdaff was actually looking to override the jupyterlab default styles for tables. We do want consistency across notebook and lab though and this would enforce that in notebook. I just don't understand why they're rendering differently given that they're both using marked... I suppose that it could be related to version inconsistency:

  • lab: "marked": "^0.3.6"
  • notebook: "marked": "~0.3"
@takluyver takluyver modified the milestones: Backlog, 5.1 Jul 3, 2017
@takluyver
Copy link
Member

@takluyver takluyver commented Jul 3, 2017

I've moved this to 'backlog' so we can get on with 5.1, but left it open in case people want to work out where the inconsistency is coming from and what we can do about it.

@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Jul 3, 2017

Ya, I've explored the marked repo and there haven't been any changes to the markdown table style for 4 years, so the cause must be a discrepancy in the way marked is used in notebook vs. lab.

@ghost
Copy link

@ghost ghost commented Jul 30, 2017

Hi gentlemen,

Has this issue been resolved? Do we now get the table content as left-aligned or centrally aligned by default, instead of the right alignment?

Thanks,
Souvik

@gnestor gnestor force-pushed the gnestor:issue-2151 branch from b5d5429 to 8644a15 Aug 1, 2017
@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Aug 1, 2017

I have just updated this PR to prevent marked from adding inline style for table cells. As a result, there will be no need for !important and this will allow users to specify custom styles in custom.css. This makes the table rendering consistent with jupyterlab (right-aligned by default).

@datasouvik If you prefer the previous styles, then you can add the following to your custom.css (located at ~/.jupyter/custom/custom.css on Unix-based systems):

.rendered_html tr, .rendered_html th, .rendered_html td {
  text-align: center;
}
.rendered_html :first-child {
  text-align: left;
}
.rendered_html :last-child {
  text-align: right;
}
@gnestor gnestor modified the milestones: 5.1, Backlog Aug 1, 2017
@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Aug 1, 2017

@takluyver Care to review/merge?

@gnestor gnestor mentioned this pull request Aug 2, 2017
11 of 11 tasks complete
Copy link
Member

@takluyver takluyver left a comment

One minor point, otherwise it looks good.

return start_tag + content + end_tag;
};
marked(text, { renderer: renderer }, function (err, html) {
// marked(text, function (err, html) {

This comment has been minimized.

@takluyver

takluyver Aug 2, 2017
Member

Do you want to clear away these commented out lines now that the new code is working?

@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Aug 2, 2017

@takluyver Thanks, merge away!

@takluyver takluyver merged commit 823e447 into jupyter:master Aug 2, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gnestor gnestor mentioned this pull request Aug 3, 2017
@gnestor gnestor deleted the gnestor:issue-2151 branch Oct 12, 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

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