Fullscreen Support

[lodev09]

This is a beta support for fullscreen. I was working on my project and would be nice to have a fullscreen feature so I added one. :smile:

Here is sample plunk: http://embed.plnkr.co/atyuBnVNktl8YSfdfPJT/preview

[acrobat]

:+1: Great feature!

I think you should rebase this pull-request against upstream-master because, you have lot's of changes here that were already commited on master, that way we won't lose changes in the progress of merging this PR in master! + it's quite difficult to see what changes exactly are related to this PR. A rebase should solve this

[lodev09]

I already rebased this and made updates based from the current v2.6.0's code

[acrobat]

hmm strange.. the commit tab shows a lot of commits which are already merged in master

[lodev09]

Indeed. But the current version I was working on was 2.6.0.

[toopay]

@lodev09 your fork hash display This branch is 23 commits ahead, 28 commits behind warning. git merge should resolve it.

[lodev09]

Try now @toopay

[acrobat]

@lodev09 please fix the classes for the switch-theme and exit-fullscreen so we add support for all the icon fonts. Thanks!

[lodev09]

Added.. Below are the default configuration

...
fullscreen: {
  enable: true,
  icons: {
    fullscreenOn: {
      fa: 'fa fa-expand',
      glyph: 'glyphicon glyphicon-fullscreen',
      'fa-3': 'icon-resize-full'
    },
    fullscreenOff: {
      fa: 'fa fa-compress',
      glyph: 'glyphicon glyphicon-fullscreen',
      'fa-3': 'icon-resize-small'
    },
    switchTheme: {
      fa: 'fa fa-adjust',
      glyph: 'glyphicon glyphicon-adjust',
      'fa-3': 'icon-adjust'
    }
  },
  theme: '' // you can put "dark" for now
}
...

[toopay]

@lodev09 Several notes :

• Reorder the fullscreen to appropriate place. Any methods which prefixed with __ are intended to use internally, while the other are exposed via markdown instance.
• If we're gonna expose this new function, then lets make it uniformal with other. showFullscreen is fine (similar with showPreview)
• Hence by nature, you will also need to maintain fullscreen state as global variable. Adding isFullscreen into class property and...
• Adding hook onFullscreen seems to be logical
• Also, can you ships the minified version instead append your changes by hand at css/bootstrap-markdown.min.css ?

Lets adding something like this properly so we doesn't break the clarity of the code as it getting bigger.

Cheers.

[lodev09]

I've renamed it setFullscreen since it's taking an argument bool mode arg.

[toopay]

Wonderful. Thanks.

One more point thou. I'm still not sure whether theme related code are required for this PR. I prefer to have the vanilla version of fullscreen feature to keep the code not having very firm ideas about how things ought to be done. I can see you're providing an option that makes it configurable, but i'm not seeing that should be within core logic, instead we can provides hook for that (onFullscreen), and let the user do whatever they need to do. Thought?

[lodev09]

I guess your right.. I just thought we could incorporate additional theme in the future (by just adding stuff in css) and let that property be the default theme of the fullscreen layout

[toopay]

Great. Lets remove any theme related block and this PR can be merged.

[lodev09]

@toopay done.

I've also added a block into the keyup event for escape. It closes the fullscreen when pressed.

[toopay]

Thanks @lodev09 !

[lodev09]

No problem!