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

Rewrite markdownviewer as a standard extension #5901

Merged
merged 5 commits into from Jan 28, 2019

Conversation

@AlbertHilb
Copy link
Member

@AlbertHilb AlbertHilb commented Jan 23, 2019

Fixes #3940, #5748

I rewrote markdownviewer as a standard extension and I added a section in the Settings Editor dedicated to it. Users now can:

  • customize how markdown is rendered setting the family and the size of the font, and the height and the width of the text line.
  • choose whether to hide the document YALM front matter (if present): a metadata block placed at the top of the document, beginning with a line of three dashes and ending with a line of three dashes (---) or three points (...).
  • change the render timeout.

Cheers

Add a section of the Settings Editor dedicated to it.
AlbertHilb added 2 commits Jan 23, 2019
Add `default` case to `switch` statement.
@ian-r-rose ian-r-rose self-requested a review Jan 26, 2019
Copy link
Member

@ian-r-rose ian-r-rose left a comment

Thanks @AlbertHilb, this looks really great. There is one command that exists in the document manager extension that now more appropriately should be here:

commands.addCommand(CommandIDs.markdownPreview, {
label: 'Markdown Preview',
execute: args => {
let path = args['path'];
if (typeof path !== 'string') {
return;
}
return commands.execute('docmanager:open', {
path,
factory: MARKDOWN_FACTORY,
options: args['options']
});
}
});

Also, this needs a pass with jlpm integrity to update some package.jsons

Other than that, this looks good to go!

@AlbertHilb
Copy link
Member Author

@AlbertHilb AlbertHilb commented Jan 26, 2019

Hi @ian-r-rose, I guess I can merge now, right?

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 26, 2019

@AlbertHilb normally we try to review and merge each other's PRs.

Can you move that one command from the docmanager-extension to the markdownviewer-extension?

@AlbertHilb
Copy link
Member Author

@AlbertHilb AlbertHilb commented Jan 26, 2019

Can you move that one command from the docmanager-extension to the markdownviewer-extension?

Sure! Sorry for the oversight. 😬

normally we try to review and merge each other's PRs.

Got it. 😊

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 26, 2019

Sure! Sorry for the oversight. 😬

No need for apologies! Just flagging it!

@AlbertHilb
Copy link
Member Author

@AlbertHilb AlbertHilb commented Jan 27, 2019

Hi @ian-r-rose, I moved the command.

Copy link
Member

@ian-r-rose ian-r-rose left a comment

This looks good @AlbertHilb. I'd also like to keep thinking about #5770, but probably after the 1.0 push we are currently undergoing.

@ian-r-rose ian-r-rose merged commit 2fde1ca into jupyterlab:master Jan 28, 2019
3 checks passed
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 28, 2019

@afshin This package restructuring will need some updates in #5845

@AlbertHilb AlbertHilb deleted the MarkdownViewer branch Jan 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants