Fix mathjax on edit and create pages #1773
Conversation
lib/gollum/views/create.rb
Outdated
| @@ -29,6 +29,14 @@ def page_name | |||
| @name | |||
| end | |||
|
|
|||
| def mathjax | |||
There was a problem hiding this comment.
Ideally we would refactor #mathjax and #mathjax_config into a module HasMath, analogous to this -- and include that in the Page, Edit, and Create views.
lib/gollum/app.rb
Outdated
| @@ -365,6 +365,7 @@ class App < Sinatra::Base | |||
| @name = wikip.name | |||
| @ext = wikip.ext | |||
| @path = wikip.path | |||
| @mathjax = wikip.wiki.mathjax | |||
There was a problem hiding this comment.
Do we not need @mathjax_config here?
There was a problem hiding this comment.
Ah, I guess not, because that is already set in the before block called before each block?
There was a problem hiding this comment.
Yes, exactly. Actually, the options are global so one could move all @mathjax to the before block where @mathjax_config is set.
lib/gollum/templates/layout.mustache
Outdated
| @@ -37,6 +37,10 @@ | |||
| {{#sprockets_javascript_tag}}editor{{/sprockets_javascript_tag}} | |||
| {{/has_editor}} | |||
| {{#mathjax}} | |||
| {{#mathjax_config}} | |||
There was a problem hiding this comment.
Does this change break existing custom mathjax configs? If I read it correctly, we currently load the default config, then load mathjax_config_path in addition. Whereas if I am right, these changes mean we only load the default config when a custom config is not enabled.
If I've misunderstood, please clarify -- the changes are a bit difficult to read!
There was a problem hiding this comment.
This is true. I didn't realize that the given config was non standard. Will revert.
|
I implemented your comments @dometto. |
|
I forgot I had to run the tests manually (for first-time PR submitters), there appear to be two failures on JRuby that I can't reproduce locally on this branch. It may be a difference in the environment but haven't figured it out yet. @fhchl if you have any idea what could be causing this please let me know |
|
Is this a problem with this pull request? Seems like the same test fails on the current master branch (https://github.com/gollum/gollum/actions/runs/1394458089) |
|
You're right, it didn't fail on |
|
Sorry for the long wait and thanks again for the fix! |
Fixes #1772
I don't have much ruby experience, but I think this patch works as expected. Apart from those that err on master, all test are running.