Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix repositioniong of the feedback popover when typing in MathQuill inputs. #1110

Merged

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Sep 1, 2024

When an answer has multiple responses and a MathQuill input for a response that is not the first one is typed in with a feedback popover open, the feedback popover does not correctly resposition. This is because currently only the first answer label is saved in the data attribute. So when the mqeditor sets up the later responses which have different answer labels, it doesn't see the feedback button. So now all answer labels for the response group are saved as a JSON encoded array. The mqeditor then searches the feedback buttons on the page to find the one with the correct answer label.

Here is a MWE for testing this:

DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl');
Context('Matrix');
$A = Matrix([ [ 0, -1 ], [ 1, 0 ] ]);
BEGIN_PGML
Enter [`[$A]`]: [_]*{$A}{10}
END_PGML
ENDDOCUMENT();

If you type in a matrix entry other than the first column of the first row when a feedback popover is open, then the popover position is not updated with the develop branch, but is with this pull request.

Note that this also occurs with MultiAnswer objects with singleResult => 1 set.

Now, I was actually trying to fix @somiaj's comment in openwebwork/webwork2#2524 (comment). I believe that this might also fix that issue. I can't test that because I can't reproduce this issue (my computer is to fast?). However, I have good reason to believe that this will fix it.

@drgrice1
Copy link
Member Author

drgrice1 commented Sep 1, 2024

Actually, I have now been able to reproduce @somiaj's issue, and this does fix it.

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

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

The issue would only happen some times, and only happens on my remote live server, not my local development server, so it felt like a timing issue. I am not able to reproduce the issue after applying this change, so it seems to fix it for me too.

…nputs.

When an answer has multiple responses and a MathQuill input for a
response that is not the first one is typed in with a feedback popover
open, the feedback popover does not correctly resposition. This is
because currently only the first answer label is saved in the data
attribute.  So when the mqeditor sets up the later responses which have
different answer labels, it doesn't see the feedback button.  So now all
answer labels for the response group are saved as a JSON encoded array.
The mqeditor then searches the feedback buttons on the page to find the
one with the correct answer label.

Here is a MWE for testing this:
```perl
DOCUMENT();
loadMacros('PGstandard.pl', 'PGML.pl');
Context('Matrix');
$A = Matrix([ [ 0, -1 ], [ 1, 0 ] ]);
BEGIN_PGML
Enter [`[$A]`]: [_]*{$A}{10}
END_PGML
ENDDOCUMENT();
```

If you type in a matrix entry other than the first column of the first
row when a feedback popover is open, then the popover position is not
updated with the develop branch, but is with this pull request.

Note that this also occurs with `MultiAnswer` objects with
`singleResult => 1` set.

Now, I was actually trying to fix @somiaj's comment in
openwebwork/webwork2#2524 (comment).
I believe that this might also fix that issue.  I can't test that
because I can't reproduce this issue (my computer is to fast?). However,
I have good reason to believe that this will fix it.
@drgrice1 drgrice1 force-pushed the another-feedback-positioning-fix branch from f8b1543 to f7a3ff3 Compare September 3, 2024 22:46
@pstaabp
Copy link
Member

pstaabp commented Sep 6, 2024

I don't see this happening, but this PR seems to still behave.

@drgrice1 I was looking at the code and I notice that on line 29 in mqeditor.js that refer to answerLabel. Does the change to answerLabels in PG.pl affect this as well?

@drgrice1
Copy link
Member Author

drgrice1 commented Sep 6, 2024

Only line 41 of mqeditor.js refers to the answerLabels in PG.pl. The other instance is a local variable.

@drgrice1
Copy link
Member Author

drgrice1 commented Sep 6, 2024

The local variable is defined on line 18 of mqeditor.js and that is what is used on line 29 (and many other places).

@Alex-Jordan Alex-Jordan merged commit ab2499e into openwebwork:develop Sep 8, 2024
3 checks passed
Alex-Jordan added a commit that referenced this pull request Sep 8, 2024
…ix-hotfix

Fix repositioniong of the feedback popover when typing in MathQuill inputs. (hotfix of #1110)
@somiaj
Copy link
Contributor

somiaj commented Sep 28, 2024

@drgrice1 Found another minor positing issue along these lines. Changing the height of a MathQuill input can affect positions of other input buttons, but only the popup of the input that changes is adjusted as a result. Here is a screenshot of the result.

image

@drgrice1
Copy link
Member Author

Hmm. I will see what I can do about that.

drgrice1 added a commit to drgrice1/pg that referenced this pull request Oct 1, 2024
drgrice1 added a commit to drgrice1/pg that referenced this pull request Oct 1, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Oct 21, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Oct 29, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Nov 25, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Nov 29, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Dec 3, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
drgrice1 added a commit to drgrice1/pg that referenced this pull request Dec 8, 2024
…input,

Don't only update the popover corresponding to the input that is
currently being typed in.

This fixes the issue reported by @somiaj in
openwebwork#1110 (comment).
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.

4 participants