Skip to content

Conversation

@mtyaka
Copy link
Member

@mtyaka mtyaka commented Jul 12, 2015

The idea is based on capa <clarification> nodes: https://github.com/edx/edx-platform/pull/6679

I decided to go with <span class="pb-clarification> instead of a custom <clarification> node because supporting clarifications inside the html child block was a requirement, but the html block is defined inside edx-platform and we aren't able to preprocess the contents of the html block from within problem-builder.

Even if we were able to get a patch for html block to support <clarification> elements upstream, I wouldn't feel very comfortable with that idea since the html block is supposed to contain plain html and making it recognize and preprocess custom elements wouldn't feel right.

The best way to implement this would probably be custom DOM elements (document.registerElement),
but browser support is not there yet and there are no reliable polyfills.

This code uses JS to process <span class="pb-clarification"> elements and transform them into the same HTML structure that's used by capa <clarification> tooltips. It relies on the LMS/Studio JS code to handle mouseovers/clicks on the tooltips, which means the tooltips don't work quite correctly in the workbench - if workbench support is required, we could copy the tooltip JS code and styles from edx-platform.

To test this, use <span class="pb-clarification">My clarification</span> inside a question of any problem type. Clarifications are also supported in MCQ/MRQ choices.

Copy link
Member

Choose a reason for hiding this comment

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

Can this be combined with mentoring.css ? Mentoring.css should really be renamed to problem_builder.css since it applies to the block as a whole. This would be a good opportunity to do so, and would reduce the number of separate CSS/JS files we are sending to the browser.

Also nit: missing newline at EOF

Copy link
Member Author

Choose a reason for hiding this comment

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

@bradenmacdonald That's a good idea, I moved the CSS rules into mentoring.css and renamed it to problem_builder.css.

@bradenmacdonald
Copy link
Member

@mtyaka This looks great. I wasn't able to think of a better approach than what you've done here. I've left some minor notes and a request for you to add a test with an HTML block. If you add that test I'll give you thumbs up; the rest of my notes are optional/nits.

Here's a screenshot of some tests I just did:
screen shot 2015-07-13 at 10 10 53 pm

One final question: When I edit an HTML block, it doesn't look so great because the <span> element has no special styling. The screenshot below is the WYSIWYG editor for the first block in the above screenshot. Did you look if there's any way to fix that?
screen shot 2015-07-13 at 10 13 54 pm

The idea is based on capa <clarification> nodes: https://github.com/edx/edx-platform/pull/6679

I decided to go with <span class="pb-clarification> instead of a custom <clarification>
node because supporting clarifications inside the html child block was a requirement,
but the html block is defined inside edx-platform and we aren't able to preprocess the contents
of the html block from within problem-builder.
Even if we were able to get a patch for html block to support <clarification> elements upstream,
I wouldn't feel very comfortable with that idea since the html block is supposed to contain
plain html and making it recognize and preprocess custom elements wouldn't feel right.

The best way to implement this would probably be custom DOM elements (document.registerElement),
but browser support is not there yet and there are no reliable polyfills.
@mtyaka mtyaka force-pushed the clarifications branch 2 times, most recently from 6ead01c to dc911fc Compare July 15, 2015 08:49
@mtyaka
Copy link
Member Author

mtyaka commented Jul 15, 2015

Thanks @bradenmacdonald! I addressed most of your comments. I also added a CSS file that gets included in the HTML editor to make the clarifications in the editor stand out a bit. Making the tooltips work inside the HTML editor would be a lot of work, so this only tweaks the style of <span class="pb-clarification"> elements s bit.

Do you want to take another look?

@bradenmacdonald
Copy link
Member

:shipit: ( 👍 )

mtyaka added a commit that referenced this pull request Jul 17, 2015
Implement tooltips/clarifications.
@mtyaka mtyaka merged commit 15945a9 into master Jul 17, 2015
@xitij2000 xitij2000 deleted the clarifications branch December 13, 2019 06:42
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

Successfully merging this pull request may close these issues.

3 participants