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

More customization #9

Closed
jlgrall opened this issue Mar 18, 2014 · 12 comments
Closed

More customization #9

jlgrall opened this issue Mar 18, 2014 · 12 comments

Comments

@jlgrall
Copy link
Contributor

jlgrall commented Mar 18, 2014

Hello again,

I would like to change a few things in Meltdown to make it more easily customizable:

  • option to auto open the preview on load
  • option to change the order between editor and preview
  • option to have editor and preview side by side
  • add a fullscreen mode
  • add a small demo.html
  • allow the user to resize the editor and the preview areas vertically

About the addToolTip() and the jQuery.qtip: a more simple solution could be to show the message in the preview area when the user clicks on TECH PREVIEW in the preview header.

I will probably add a simple JS API to interact with the meltdown editor. It will probably be something like a jquery-ui widget, but simpler and not requiring jquery-ui. Thus I will need to refactor parts of the code.

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 18, 2014

Is there any reason to the preview being hidden by default ?

I will probably change it and add the option: autoOpenPreview = true by default.

@iphands
Copy link
Owner

iphands commented Mar 19, 2014

I like the options. Feel free to add them all... but I would rather have the default be preview closed.

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 24, 2014

To me it doesn't make sense to use a markdown editor if we don't want to use the preview. Why not use a text editor in this case ? If it is because the preview takes too much space, I am adding options to choose how big it should be (or even allow the size to adapt to its content).

I made some good progress. You can see here: https://github.com/jlgrall/Meltdown/tree/customize

Main changes:

  • Added a demo.html page
  • Added many init options (see beginning of jquery.meltdown.js) like:
    • specify order of controls in toolbar
    • auto open preview
    • preview can adapt to its content dynamically
    • use of custom parser
  • Preview updates are debounced while typing
  • Fullscreen mode
  • Editor and preview can be resized with the mouse
  • Added support for a resize handle (for now working by dragging the preview header)
  • Click on TECH PREVIEW shows the warning in the preview

Notes:

  • Requires jQuery 1.8 because I use the progress event callback.
  • Should still work in older browsers, though I didn't test this yet.
  • Removed optional dependencies on jQuery-ui and jQuery.qtip.
  • Added dependency on element_resize_detection.js
  • The parser must now be loaded before jquery.meltdown.js

API changes:

  • Every meltdown is now backed by a Meltdown object. This is transparent to the user.
  • Functions can be called with: m1.meltdown("togglePreview", args...)
  • Options can be changed with: m1.meltdown("options"[, newValue])
  • It is possible to access the backing Meltdown object with: m1.data("Meltdown"), giving access to most internal properties for debugging or deeper customization.

@iphands
Copy link
Owner

iphands commented Mar 24, 2014

  • Requires jQuery 1.8 because I use the progress event callback.

Yikes, I really need legacy jQuery support for the time being.
Can we write a bit of code (even if it is a hack) as a workaround for 1.7.2?

  • Should still work in older browsers, though I didn't test this yet.

Same as above, unfortunately IE8 is our lowest supported browser atm.

  • Removed optional dependencies on jQuery-ui and jQuery.qtip.

Sweet!

  • Added dependency on element_resize_detection.js

Works for me.

  • The parser must now be loaded before

Also works for me

After this is merged in I will probably refactor to make this an AMD/require.js module. It is the way we are rolling these days.

@iphands
Copy link
Owner

iphands commented Mar 24, 2014

"""To me it doesn't make sense to use a markdown editor if we don't want to use the preview."""
This is one thing I am going to stand firm on. Until #1 is resolved I am going to keep the box hidden until the user clicks to show a preview.

Even after issue #1 is fixed I still feel that the toolbox alone is an added benefit, and expanding the preview is something I would rather not see as the default option. Having the auto_open option should be all we need.

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 24, 2014

Thanks for the feedback.

I will leave the auto open to false then.

For IE 8, I will see what I can do. Unfortunately, I only have access to an IE 7 on WinXP as dual-boot, and no other IE. I will keep using cross-browser code, and then, when I have time, I will try to test it on IE. But it is at the bottom of my priority list for now...

For jQuery 1.7.2, I think I could simply disable the preview slide animation in fullscreen mode. The progress callback is used to slide the editor in the opposite direction from the preview.

Making it an AMD/require.js module is a good idea :)

Another question, do you mind if I detab the code in the global self-executing function ? I usually do this for all my scripts that are in a self-executing functions. It saves horizontal space and file size.

As a possible workaround for #1, I considered the possibility of using webworkers for the parsing. The interesting point is that webworkers can be interrupted at any time if they timeout. When they time out, we can display a warning and a button "Force parsing" in case the user has a slow computer. Then a button "Interrupt parsing". This warning could be displayed in the preview, just like the "TECH PREVIEW" warning is displayed. Well, I haven't decided yet...

I never saw the infinite loop from #1. Does it happen often ? Do you know a markdown text that triggers it ?

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 24, 2014

Another idea: sync the editor and preview scroll positions.

I think a way to go could be by having the parser add to each generated element, its source line number: tanakahisateru/js-markdown-extra#34

Any better idea ?

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 29, 2014

I made a layout change, I hope you are ok with it, otherwise just tell me...

While doing the sidebyside feature, I saw that the toolbar could become really too small, which resulted in accessibility problems but also in layout bugs. So I decided to put the toolbar at the very top. It stays always over the editor and when the editor and the preview are side by side, the toolbar stays at the same place over both the editor and the preview.

There are still some troubles when the toolbar is not large enough: the wrapped controls are pushing the following content downwards. And I cannot overflow: hidden the toolbar, because of the submenus...

@jlgrall
Copy link
Contributor Author

jlgrall commented Mar 30, 2014

Hello again, I am now at the end of what I wanted to add for this PR.

Last changes:

  • Add option to auto scroll preview while typing at the bottom
  • Press Esc key to exit fullscreen
  • Controls separator defined by the controls list (instead of being defined by the CSS)
  • If the toolbar is too small, overflowing controls are hidden
  • Added sidebyside mode
  • Added resize handle that works vertically and horizontally

Notes:

  • Requires jQuery 1.7.2 (degraded experience and no sidebyside mode), jQuery 1.8+ recommended
  • Layout changed: toolbar is always on top, then comes the editor, then the resize handle, then the preview.

To be done:

  • Test on IE 8 and 9, including tests with jQuery 1.7.2 (I cannot test on IE 9, I will see what I can do for IE 8)
  • Update the documentation: add the new options and API, update gh-pages
  • Update version to 0.2 ?
  • Remove lte-ie7.js ? (Since IE 8 is the lowest supported browser)
  • Add git tags: v0.1 and v0.2

@jlgrall
Copy link
Contributor Author

jlgrall commented Apr 8, 2014

I tested and fixed the IE 8 issues. It now works on IE 8 with jQuery 1.7.2 or newer.

I cannot test with IE 9.

In the future, removing support for IE 8 and jQuery 1.7.2 should be easy because the relevant code is mostly isolated from the main code.

I am going to make rangyinputs a required dependency, because the toolbar is pretty useless without it and it is a small dependency anyway.

I am currently updating the documentation (the README.md file).

@jlgrall
Copy link
Contributor Author

jlgrall commented Apr 9, 2014

As I didn't receive any answer for the last 2 weeks, I decided to go on. I hope you agree with what I did.

It is now ready for a pull request. You can add the AMD/require.js module code and add that info in the README.md and the changelog.

I added the git tag v0.1 to the last commit before I started this branch. That was the last version that was compatible with IE 6-7 and which had not too much incompatible differences with your initial commit.

When you merge this branch, you should then add the v0.2 git tag and update the release date in the README.md changelog and in the header of jquery.meltdown.js

Have a good day.

Edit: info for AMD/require.js module:

  • currently the only two exported variables are jQuery.meltdown and jQuery.fn.meltdown
  • I reduced the globals to only jQuery and addResizeListener

@jlgrall jlgrall mentioned this issue Apr 9, 2014
@iphands
Copy link
Owner

iphands commented Apr 12, 2014

Cool. Thanks I will have a look and merge sometime soon.

@iphands iphands closed this as completed Apr 13, 2015
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

No branches or pull requests

2 participants