search

numbas / Numbas

A completely browser-based e-assessment/e-learning system, with an emphasis on mathematics
http://www.numbas.org.uk
Apache License 2.0
199 stars 118 forks source link

Fix #814 and #859 #903

Closed alisahq closed 1 year ago

alisahq commented 2 years ago

Hi Christian,

We tried to fix issues #814 and #859 . Could you please take a look at our change? Thanks and we appreciate your help!

Best, Alisa

christianp commented 2 years ago

Thanks! I'm on leave until the 19th. I'll look when I get back.

christianp commented 2 years ago

I'm really sorry I didn't look at this earlier - after my leave we had some strike days, and then I completely forgot about this!

I think you've defined the add_credit_if function in the wrong way: it's a JavaScript function, but I meant for you to add a JME function which could be used from a marking algorithm. I'm going to expand the description of issue #859 to make that clearer.

It looks like you put the button in OK, but you didn't need to add a unit test (and in fact, that test won't pass because the JME unit tests don't use the theme). There's also a stray comment that I can't see the point of.

You added a CSS rule for the selector .btn: this will apply to every element with .btn in the page, which could be quite a few! I think the right thing would be to add a class display-options to the button element, and use the selector .btn.display-options. The big margin you've set will not look good on small screens; I think you could just set an all-round margin: 1em and it'd look OK.

christianp commented 1 year ago

I'm closing this now, because I've dealt with #814 myself, and the change for #859 really needs a new PR.