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

Center the RadioMultiAnswer error messages. #1135

Conversation

drgrice1
Copy link
Member

The left aligned style worked well with the attempts table, but does not look so good with the new feedback buttons. Having the messages centered looks better now since everything else is centered.

A problem to test this with is attached. The problem is set up to give a message for any answer.
appendMessageMinimal.zip

@somiaj
Copy link
Contributor

somiaj commented Oct 16, 2024

Think MulitAnswer should be updated as well, their tags still left align error messages.

@somiaj
Copy link
Contributor

somiaj commented Oct 16, 2024

Though I guess MultiAnswer is wrapped in a table, so it isn't quite the same situation.

@drgrice1
Copy link
Member Author

The RadioMultiAnswer is also wrapped in a table. I also had thought that MultiAnswer had this issue also, but in a quick test I didn't see the issue. I will look at it again.

@drgrice1
Copy link
Member Author

I see. So I rarely (if ever) use the MultiAnswer setMessage method. Instead I use the addMessage method (with singleResult set). The messages added by addMessage (which go into the single_ans_messages array) are placed in consecutive divs instead of the table. Those messages are centered. So that is why my brief test didn't show an issue.

I tested using the setMessage method instead, and see that those messages aren't centered. I think the only thing that can be done here is to set width:100% on the table.

@drgrice1
Copy link
Member Author

I added another commit to this pull request that sets width:100% on the MultiAnswer error message table. It also switches to using the PGbasicmacros.pl tag method for generation of the HTML, and removes the deprecated attributes that were used. Note that the ArrayLayout css class already does what the deprecated attributes were doing.

@drgrice1 drgrice1 force-pushed the radio-multi-answer-message-style-tweak branch from 773f35e to 4f21da2 Compare October 16, 2024 11:47
@somiaj
Copy link
Contributor

somiaj commented Oct 25, 2024

I notice a few things testing this with parserMultiAnswer. First minor thing, setMessage(1, 0);, doesn't actually show a message and I was confused for a while until I caught that (was setting messages in a loop). Probably not a big deal, I doubt anyone would set the message to zero outside of testing.

Second, I don't think the table width being 100% looks correct. Here is what I'm seeing.

scrot1

If I use the inspector to remove the width from the table, but set the mx-auto class instead it looks better to me. Though I assume we need to not use the bootstrap class but its equivalent in pg.

scrot2

Last, I use Value->Error('message') out of habit (need to switch to addMessage), and those are still left aligned which also looks off to me.

scrot3

@somiaj
Copy link
Contributor

somiaj commented Oct 25, 2024

Here is the test case I was using.

DOCUMENT();
loadMacros(qw(PGstandard.pl PGML.pl parserMultiAnswer.pl));

$ma = MultiAnswer(0 .. 3)->with(
    singleResult => 1,
    checker      => sub {
        my ($c, $s, $self, $ans) = @_;
        Value->Error('Test');
        for my $i (0 .. 3) {
            $self->setMessage($i + 1, (A .. D)[$i]);
        }
        return $score;
    }
);

BEGIN_PGML
0 = [_]{$ma}, 1 = [_]{$ma}, 2 = [_]{$ma}, 3 = [_]{$ma}
END_PGML
ENDDOCUMENT();

@drgrice1
Copy link
Member Author

I switched to centering the tables for the messages, instead of making them width 100%.

As to the issue with setMessage(1, 0), that has nothing to do with this pull request. Also, I don't really see that as being something that we need to support even for test usage.

@drgrice1
Copy link
Member Author

I also centered the error messages that occur for parserMultiAnswer.pl.

@drgrice1 drgrice1 force-pushed the radio-multi-answer-message-style-tweak branch from eb3ba63 to 44b1afe Compare October 25, 2024 19:42
@drgrice1
Copy link
Member Author

The error messages for parserRadioMultiAnswer.pl are also centered.

@somiaj
Copy link
Contributor

somiaj commented Oct 25, 2024

I agree, no need to deal with a 0 value here, just something that initially confused me so I pointed it out.

@drgrice1 drgrice1 force-pushed the radio-multi-answer-message-style-tweak branch from 44b1afe to e59855f Compare October 29, 2024 19:42
The left aligned style worked well with the attempts table, but does not
look so good with the new feedback buttons.  Having the messages
centered looks better now since everything else is centered.
Use styles instead.

Also switch to using the `tag` method.
@drgrice1 drgrice1 force-pushed the radio-multi-answer-message-style-tweak branch from e59855f to c974bda Compare November 25, 2024 02:42
@Alex-Jordan Alex-Jordan merged commit d52e7fa into openwebwork:develop Nov 29, 2024
3 checks passed
@drgrice1 drgrice1 deleted the radio-multi-answer-message-style-tweak branch December 17, 2024 12:50
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