search

RunestoneInteractive / RunestoneComponents

Packaging of the Runestone tools for publishing educational materials using github pages
http://runestoneinteractive.org
Other
101 stars 225 forks source link

Updates and additional features for select toggle functionality #1189

Closed vqum closed 3 years ago

vqum commented 3 years ago

Apologies that this looks like a lot of changes, but they are all related to each other and involve the same files, so I figured that they should reasonably be moved together.

Overview of changes:

  1. Relabel "Parsons" as "Mixed-Up Code" and "Active Code" as "Write Code"
  2. Move preview panel buttons from the bottom to the top
  3. Add toggle option "lock"
  4. Add toggle option "transfer"

1. Relabel "Parsons" as "Mixed-Up Code" and "Active Code" as "Write Code" A group of U-M students conducted user testing using the select toggle functionality, and one of their findings was that the question type label within the toggle dropdown menu should match the wording that has been used elsewhere in Runestone textbooks. image image

2. Move preview panel buttons from the bottom to the top Another finding from the above mentioned user testing was that the toggle preview panel's buttons being at the bottom made them difficult to locate. This was especially true for long problems, such as Parsons problems whose source and answer spaces become stacked vertically resulting in the need for scrolling to get to the bottom and see that there are actionable buttons. Additionally, the buttons at the bottom could be obscured by the privacy policy banner. image image

3. Add toggle option "lock" @barbarer asked for the ability to require that students try to solve a specific question (in other words, only that specified question would be graded) but that the other questions would be made available for the students to use as reference in working on the specified question. For achieve this, the option "lock" has been made available on the :toggle: setting. When applied, text is added to the first option on the dropdown menu describing that only that question would be graded, and the "Select this Problem" button is removed from the preview panel. This results in a select toggle question in which the student is able to pick other questions options from the dropdown menu to preview them, but is not able to set those other questions nor have the grading table be updated. image image image @bnmnetp The question about displaying a question in a separate window was related to this - some questions like Parsons problems take up too much screen space in the preview panel, obscuring the current question underneath. For a student to be able to work on the current question and be able to see the Parsons problem as a reference simultaneously, do you think it would be possible to implement something like a button "Open Preview in Separate Window" on the preview panel that accomplishes this?

4. Add toggle option "transfer" Option "transfer" made available on the :toggle: setting. When applied, and when the current question is of type Parsons and the dropdown-selected question is of type active code, the button "Transfer Mixed-Up Code to Write Code" is added to the preview panel. Clicking this button extracts the text with indentations as it's currently set in the answer space of the Parsons question and pastes it into the active code. Any preexisting code within the write code is overwritten. image image image image image

bnmnetp commented 3 years ago

Thanks Vincent,

A few of comments. Hopefully @barbarer will join in as well.

I think "Write code" and "mixed up code" problems are local terminology at UMich. I've never in my life referred to an activecode problem as a "write code" problem nor have I heard anyone but @barbarer use that term. Maybe @bhoffman0 ?? I always call parsons problems "parsons problems". So maybe there is a way we can configure that similar to the localization we do for other elements? I could see that there might be other more preferred labels for the use case of :toggle: lock such as Hint 1, Hint 2, Hint 3 rather than labeling them with their ids and question types?? So maybe a parallel option of :labels: would all the author to call the choices whatever they want, with the fallback of just using the type and label??

I'm 100% against popups. Lots of browsers prevent them by default and they count against you in all sorts of ways by sites that try to grade you on privacy and efficiency. I can only imagine all the bug reports we would get when people click the button and no window pops up because their browser prevented it. Its kind of like the issue we have with timed exams and navigating away from the page. We are doing it for a good reason but browsers are doing everything they can to prevent us from being nice because it is most often used for evil.

Maybe we can update the preview window to be more like the modal reveal that is resizable and moveable? Or maybe we should just position it full width in the DOM above or below the actual question. We can keep the shadowing and background we have now to set it apart but just make it larger and change its position?

The transfer idea is super cool. Love it. As well as the option.

barbarer commented 3 years ago

In our ebooks we use the terms Mixed-up code (not Parsons) so I like the idea of using :labels: to provide a name or by default use the type and divid.

We are talking about different ways to handle popping up the Parsons when the student is struggling while writing code. One way is to make the preview panel bigger, but we also want people to be able to look at the Parsons while writing the code. We could also change the Parsons problem to a single column format when the width is smaller.

vqum commented 3 years ago

I've added labels as a setting to :toggle:, but I'm encountering an issue that including any spaces within this string seems to result in the text being cut off, for example: :toggle: labels{parsonsexample1, parsonsexample2, activecode1} results in the reading in of labels{parsonsexample1, while :toggle: labels{parsonsexample1,parsonsexample2,activecode1} results in the reading in of labels{parsonsexample1,parsonsexample2,activecode1} This seems to be occurring before any of the JavaScript code that I've written because adding console.log to output either of these values for toggleOptions produces only the section of text before the first space detected within the text image Any ideas why it might be doing this and whether I could resolve this?

Additionally, for the transfer button, I've relabeled it to display as "Transfer Parsons Mixed-Up Code to Active Write Code" as an attempt at compromise, but I don't like how wide that makes the button itself. I welcome any ideas for this as well.

barbarer commented 3 years ago

Just label it "Transfer Solution" they will figure it out.

Dr. Barbara Ericson Assistant Professor, School of Information University of Michigan

On Tue, May 11, 2021 at 6:40 PM vqum @.***> wrote:

I've added labels as a setting to :toggle:, but I'm encountering an issue that including any spaces within this string seems to result in the text being cut off, for example: :toggle: labels{parsonsexample1, parsonsexample2, activecode1} results in the reading in of labels{parsonsexample1, while :toggle: labels{parsonsexample1,parsonsexample2,activecode1} results in the reading in of labels{parsonsexample1,parsonsexample2,activecode1} This seems to be occurring before any of the JavaScript code that I've written because adding console.log to output either of these values for toggleOptions produces only the section of text before the first space detected within the text [image: image] https://user-images.githubusercontent.com/40499850/117893049-b01dbc80-b287-11eb-9cbe-0c2565db0859.png Any ideas why it might be doing this and whether I could resolve this?

Additionally, for the transfer button, I've relabeled it to display as "Transfer Parsons Mixed-Up Code to Active Write Code" as an attempt at compromise, but I don't like how wide that makes the button itself. I welcome any ideas for this as well.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RunestoneInteractive/RunestoneComponents/pull/1189#issuecomment-839244516, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKOZ7M2HM4R2HM4B4TEWCB3TNGW6DANCNFSM43XYXJ7A .

bnmnetp commented 3 years ago

That does not make sense. Have you stepped through it in the debugger?

bnmnetp commented 3 years ago

Have you looked at the generated html to make sure that all of the options are part of the page?

bnmnetp commented 3 years ago

We have lots of other places where we provide a list of things in a directives.unchanged. Seee the :include: option in activecode for example. There we strip out the comas before generating the html.

bnmnetp commented 3 years ago

Actually I just pulled it down and I think you are just missing quotes around your list. I checked the html that was built and here is what I see:

<div class="runestone alert alert-warning sqcontainer">
                    <div data-component="selectquestion" id=dynamic_toggle_1 data-questionlist='over_ac_example1, morning' data-toggle=true data-toggleoptions=Example1, Example2>
                        <p>Loading ...</p>
                    </div>

In this case the parser to get the data element is only going to see Example1, any attribute value that has a space in it must be quoted.

vqum commented 3 years ago

Thanks Brad. The quotations in selectone.py have now been escape keyed so that it sends over the entirety of the text including spaces: image image image

Leading and trailing spaces in the labels are also trimmed: image

bnmnetp commented 3 years ago

@vqum

I like how you have it working so that it defaults correctly if you leave one label blank!

I don't think I like the syntax of the :toggle: option, I think it would be better to break it up into :toggletype: and :togglelabels: I think we may be asking for trouble as bootstrap uses data-toggle for several things and I do not want to conflict. Also I think it is just more complicated for authors to use the one option for multiple things...

It looks like there is one minor conflict with the main branch. Can you check and resolve it?

vqum commented 3 years ago

Separated into :toggle: and :togglelabels:, with the presence of either (or both) causing the select to become a toggle question. The text input into :toggle: is passed into the data-toggleoptions attribute to avoid the bootstrap conflict. image

This is in conjunction with RunestoneServer pull request https://github.com/RunestoneInteractive/RunestoneServer/pull/1656 to update ajax.py get_question_source accordingly.

I believe I also resolved the conflict with the main branch.

I tested the following cases which use preexisting questions in CSAwesome, and all of them rendered the question as expected:

vqum commented 3 years ago

For this conflict image

I simply want to replace component-preview with toggle-preview, and I'm not sure why Git has highlighted this as a conflict. image

After I click 'Mark as Resolved', am I supposed to click 'Commit merge'? I don't want to accidentally preemptively merge my code into the master if that's what that button will do. image

Besides that, this should be good to go at least on my end.

bnmnetp commented 3 years ago

Its because git has decided that someone else changed that line. That can happen when others are also making changes to the same file.

I can fix the conflict. I see now that you want it to be toggle-preview