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

Added position and slide tracking for reveal.js #181

Conversation

arBmind
Copy link
Contributor

@arBmind arBmind commented Oct 2, 2014

The reveal.js editing feature is promising.
Unfortunatly the preview jumped to the first slide on every edit. And for large presentations it's hard to find the slide in the markdown.

I added both features with this pull request.
The code is a little rough, as everything is done in the mainwindow. But the functionality is there and works.

@cloose
Copy link
Owner

cloose commented Oct 4, 2014

Hi!

Thanks a lot of this contribution!

This works great but as you said the code is a litte rough. Are you interested to work on this a little more? It would be great if most of this code could be moved to the app-static library. Maybe even with some unit tests added. 😄

Some ideas:

  • Maybe it's possible to create an interface for this functionality. For normal Markdown documents it could be implemented with a Null Object that does nothing. This could reduce the number of if (options->markdownConverter() == ...) blocks.
  • Brainstorm naming idea SlideTextLineSynchronizer
  • Use signal/slots to connect ui elements (Text editor & webview) with the new separate class

Christian

@cloose cloose self-assigned this Oct 4, 2014
@cloose cloose added this to the v0.11.0 milestone Oct 4, 2014
{
if (options->markdownConverter() == Options::RevealMarkdownConverter) {
ui->webView->page()->mainFrame()->evaluateJavaScript(
"(function(){"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this javascript code could be put into PresentationTemplate class. See the scrollbar synchronization in HtmlTemplate class for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that already. But this would also be part of any html export.

@arBmind
Copy link
Contributor Author

arBmind commented Oct 4, 2014

Thanks a lot for your feedback.

I have not placed the code anywhere special because I do not know where you want to go. (The roadmap is a dead link.)

We have:

  1. Normal Markdown documents
    • All kinds of styles
    • Editing tools
    • Very rough scroll synchronisation
  2. Reveal JS preview
    • Styles that fight with reveal.js (I have to switch to Github)
    • No special tools (toc is broken ...)
    • Highly coupled scroll support (But normal markdown to html could be skipped)

If we are serious about reveal.js - we need a lot of new abstractions.
I tend to shoot over the goals, with that.

cloose added a commit that referenced this pull request Oct 6, 2014
Moved the logic for the line to slide mapping from the MainWindow into the separate SlideLineMapping class and refactored the code to be more "Qt-like".

Refactoring of #181
@cloose
Copy link
Owner

cloose commented Oct 12, 2014

@arBmind
I tried to refactor the view synchronization and moved it into separate classes. Could you take a look at PR #184? What's your opinion?

Thanks!
Christian

@cloose cloose merged commit 38b6418 into cloose:develop Oct 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants