Skip to content

fix capa textline to properly use inline if specified when using math preprocessor - #2869

Merged
ichuang merged 3 commits into
masterfrom
bugfix/ichuang/textline-inline-for-jspre
Mar 10, 2014
Merged

fix capa textline to properly use inline if specified when using math preprocessor#2869
ichuang merged 3 commits into
masterfrom
bugfix/ichuang/textline-inline-for-jspre

Conversation

@ichuang

@ichuang ichuang commented Mar 8, 2014

Copy link
Copy Markdown
Contributor

When using the dynamic math javascript preprocessor feature, textline does not honor the "inline" flag, because of a div which is fixed to be display:block.

This PR fixes that bug, using the same approach as previously employed (https://github.com/edx/edx-platform/pull/845).

1 line, 12 character fix.

@chrisndodge ?

@chrisndodge

Copy link
Copy Markdown
Contributor

@ichuang

ichuang commented Mar 8, 2014

Copy link
Copy Markdown
Contributor Author

Thanks, @chrisndodge. I see where the test is failing; let's see if this fixes it.

@ichuang

ichuang commented Mar 8, 2014

Copy link
Copy Markdown
Contributor Author

tests passing now.

@chrisndodge

Copy link
Copy Markdown
Contributor

@ichuang thx. Likewise, could you forward some sample capa definition so I can try locally? Thx.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This trailing space seems error-prone. Best is to treat the class attribute as a set of space-separated words. That's awkward to do in XPath, but I think this would be a better fix: //div[contains(@class, "text-input-dynamath_data")]

@chrisndodge

Copy link
Copy Markdown
Contributor

Pinging @Lyla-Fischer on this one as well.

@ichuang

ichuang commented Mar 10, 2014

Copy link
Copy Markdown
Contributor Author

@chrisndodge change to test made; added an additional new test to ensure inline appears on that element.

@ichuang

ichuang commented Mar 10, 2014

Copy link
Copy Markdown
Contributor Author

Here is a sample capa XML code fragment:

<p>
  <p style="display:inline">[mathjaxinline]\displaystyle [J_ z, H] =[/mathjaxinline]</p>
  <customresponse cfn="test_ket" inline="1" expect="0" options="samples='c,h,m_J,J@1,1,1,1:20,20,20,20#20'" cfn_extra_args="options">
    <textline size="60" correct_answer="0" inline="1" math="1" preprocessorClassName="MathjaxPreprocessorForQM" preprocessorSrc="/static/js/mathjax_preprocessor_for_QM.js"/>
  </customresponse>
</p>

It needs a python check function, and the js preprocessor code as well; you can make dummy ones of these. The 6.002x course source uses such functions and js.

@Lyla-Fischer

Copy link
Copy Markdown

This is not going to change any defaults - right? Any element that does not have the style="display:inline" will continue rendering in exactly the same way that it was rendering before?

@ichuang

ichuang commented Mar 10, 2014

Copy link
Copy Markdown
Contributor Author

correct

@Lyla-Fischer

Copy link
Copy Markdown

Seems fine to me then.

ichuang added a commit that referenced this pull request Mar 10, 2014
…jspre

fix capa textline to properly use inline if specified when using math preprocessor
@ichuang
ichuang merged commit c543dbb into master Mar 10, 2014
@ichuang

ichuang commented Mar 10, 2014

Copy link
Copy Markdown
Contributor Author

thanks

@ichuang
ichuang deleted the bugfix/ichuang/textline-inline-for-jspre branch March 10, 2014 14:47
@Lyla-Fischer

Copy link
Copy Markdown

The sample code fragment provided here does not work on stage.

@ichuang

ichuang commented Mar 12, 2014

Copy link
Copy Markdown
Contributor Author

it won't work unless you modify it to include an appropriate python check function and js math preprocessor.

@Lyla-Fischer

Copy link
Copy Markdown

Ah - okay. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants