OC-1387 Always show tooltips feedback if tips are present.

[jbzdak]

Testing instructions:

Problem Builder

MRQ

Create following MRQ block, and verify that behavior is unchanged:

1.That when you create submit you get question feedback: "Thank you for answering!"
2. And after clicking check-mark you get tooltip.

<problem-builder display_name="Default Title" weight="1" mode="standard">
    <html_demo>
        <p>Please answer the questions below.</p>
    </html_demo>

    <pb-mrq name="mrq_1_1" question="What do you like in this MRQ?" message="Thank you for answering!" required_choices='["gracefulness","elegance","beauty"]'>
        <pb-choice value="elegance">Its elegance</pb-choice>
        <pb-choice value="beauty">Its beauty</pb-choice>
        <pb-choice value="gracefulness">Its gracefulness</pb-choice>
        <pb-choice value="bugs">Its bugs</pb-choice>

        <pb-tip values='["gracefulness"]'>This MRQ is indeed very graceful</pb-tip>
        <pb-tip values='["bugs"]'>Nah, there aren't any!</pb-tip>
    </pb-mrq>

    <pb-message type="completed">
        <p>Congratulations!</p>
    </pb-message>
    <pb-message type="incomplete">
        <p>Still some work to do...</p>
    </pb-message>
</problem-builder>

MCQ

Create following MCQ block, and verify that behavior is changed.

1. When you select a,b or d you get a tooltip straight away
2. When you select c you get question level feedback

<vertical_demo>
    <html_demo><h1>Problem Builder Demo</h1></html_demo>

    <problem-builder display_name="MCQ With Tips" weight="1" mode="standard">
        <html_demo>
            <p>Like the MRQ above, multiple choice and rating questions can also provide feedback based on which answer was selected and whether the answer is correct or not. Here's an example:</p>
        </html_demo>

        <pb-mcq name="subjunctive"
          message="This is question level feedback"
          question="Which sentence correctly uses the subjunctive case?" correct_choices='["d"]'>
            <pb-choice value="a">To buy or not to buy, that is the question.</pb-choice>
            <pb-choice value="b">Renting gives you more flexibility.</pb-choice>
            <pb-choice value="c">If I was you, I'd buy the house.</pb-choice>
            <pb-choice value="d">If I were you, I'd rent the house.</pb-choice>
            <pb-tip values='["a","b"]'>This sentence is not discussing hypotheticals or impossibilities.</pb-tip>
            <pb-tip values='["d"]'>Correct. "was" has become "were" to indicate the subjunctive case.</pb-tip>
        </pb-mcq>

        <pb-message type="incomplete">
            <p>Read the tip above and try again.</p>
        </pb-message>

        <pb-message type="completed">
            <p>Nicely done. If I were you, I'd be proud of myself.</p>
        </pb-message>
    </problem-builder>

    <html_demo>
        <h1>See the details</h1>
        <p>Check out the XML source below to get an idea of how these mentoring questions were set up and what kind of options authors have available.</p>
    </html_demo>
</vertical_demo>

Step Builder

It is not affected AFAIK, as doesn't display tooltips at all, and can optionally display question level feedback messages (on the review block).

[jbzdak]

This is unrelated, but made CircleCi tests to fail (strangely it passed on my system) --- {{ is actually an escape sequence that means { in str.format.

[bradenmacdonald]

Yeah, I think these were originally copied from a django template. Not sure why they worked before this change.

Incidentally, the mode parameter is deprecated, and we will eventually remove support for it.

[bradenmacdonald]

Tested and it works well, but I had some questions and think we need a bit more test coverage.

[jbzdak]

These two were removed on purpose. To test questionaire I needed to have options with tooltips and without them. So I removed tooltips from selected choices. Other tests seem to be not affected.

[bradenmacdonald]

@jbzdak Just had two minor notes. Also apparently this needs a rebase now :/ Please address those last two little things and rebase. I'll test the rebased version one last time and give you +1.

[jbzdak]

Unrelated tests failed locally, as there were trying to click on invisible elements, this fixed the issue for me, we can either leave it for the merit of making tests more robust, or just remove.

Really I feel that selenium should be doing this scrolling, but apparently it isn't.

[bradenmacdonald]

@jbzdak Selenium should be doing this scrolling - is it possible that the elements in question were just hidden (in the process of becoming visible), and this scroll_to has a side effect of fixing it since it acts as a short delay?

These scroll_to calls seem very hacky, and I'd rather find a more direct way of addressing the problem.

Please try this way of fixing it instead (which is a nice cleanup, if it works): 351705c

[bradenmacdonald]

@jbzdak The problem with the currently-failing tests is ultimately caused by the fact that you are calling self.load_scenario(scenario) twice within the same test_feedback_persistence_tips test case, which as you've found does not work. Unfortunately as is often the case with XBlock SDK, you'll see all kinds of random ORM/DB/txn error messages, so this type of thing is hard to diagnose.

All you need to do to fix the issue is replace:

self.reload_student_view()
mentoring = self.load_scenario(scenario)

with

mentoring = self.reload_student_view()

[jbzdak]

Logic used to display message was changed, so take a second glance here (it works as before). I just wanted you to know that there are new changes here.

[bradenmacdonald]

Thanks - looks good.

[bradenmacdonald]

Looking good! I just want a cleaner fix for those tests - I provided a suggestion that may do the trick. Then this will be good to go.

[jbzdak]

@bradenmacdonald I have integrated your fix, and everything works fine now.

[bradenmacdonald]

👍

[jbzdak]

Thanks! Merging I'll squash the commits