damithc / testrepo3

0 stars 0 forks source link

mashup.jsp: Instructor Feedback Preview as Student Page : no combo box to select recipient #266

Open damithc opened 10 years ago

damithc commented 10 years ago

From dam...@gmail.com on February 22, 2014 13:53:03

Not sure if this is only in the mashup page or it is relevant to the actual page as well.

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1633

damithc commented 10 years ago

From arnold.k...@gmail.com on February 21, 2014 22:25:27

It is only in mashup. It's because mashup only loads the frameBodyWrapper and the dropdown is located at the frameheader. Trying to load the whole page in mashup causes the page to be messed up (see screenshot at issue 1625 ) which I'm not sure yet why (maybe css/javascript issue?)

damithc commented 10 years ago

From arnold.k...@gmail.com on February 21, 2014 22:28:52

Oh, and the dropdown box is only at Eval Preview, not Feedback Preview. Fro Feedback Preview the coice it before the preview page at Feedback Edit. This was because Feedback Preview was implemented that way first then as Eval doesn't have an edit page to place the choice, the choice got placed in the preview page.

damithc commented 10 years ago

From dam...@gmail.com on February 22, 2014 00:19:38

No, not that dropdown. This is when the student have to give feedback to 5 students and he/she has to choose which 5 students from a dropdown. currently, it only shows five 'To' without the dropdown

damithc commented 10 years ago

From dam...@gmail.com on February 22, 2014 00:46:04

Seems to be in the mashup page only. It is a case of students being asked to give feedback for 5 other students, but the class size is exactly 5. So there is no need for drop down box. The names should be shown instead. See attached.

Attachment: missing names.png

damithc commented 10 years ago

From arnold.k...@gmail.com on February 26, 2014 22:15:59

The student submit page (not preview) in mashup also have this problem. Instructor might also have the problem (need to check, current test data has no question with multiple receiver)

damithc commented 10 years ago

From arnold.k...@gmail.com on March 13, 2014 19:51:29

Investigated and found why this problem occurs:

When the number of recipient is small enough and the dropdown is not required, the dropdown is hidden by the jsp. Then, on page load a javascript will place the recipient name in place of the dropdown if it is hidden. Mashup however does not have this javascript so the name is not shown.

I can always reduce the number of students to give feedback to so that the dropdown will show? The content of the dropdown will be wrong because it's also managed by javascript though

Cc: dam...@gmail.com

Attachment: Untitled picture.png

damithc commented 10 years ago

From dam...@gmail.com on March 13, 2014 20:47:15

Ideally, both form should show up in the mashup because we want to see all different UI combinations in the mashup :-)

damithc commented 10 years ago

From edi33...@gmail.com on March 20, 2014 08:09:53

Hello,

If I have understood correctly what was the issue, I believe that I have fixed it.

I have put the code review link bellow: https://codereview.appspot.com/78200044/ Also I have attached a print screen with the running test :-)

Have a nice day!

Attachment: mashupFeedbackComboBox.jpg

damithc commented 10 years ago

From arnold.k...@gmail.com on March 20, 2014 19:47:18

Hi Edi,

What you did is incorrect as for this page we only want to show the dropdown if the number of recipients available is lmore than the number of feedback the giver need to give. When we have less/equal number of recipients as in this case, the page should show "To: Recipient Name" without the dropdown.

as disscussed in comment #7, in the current data what happens is there is less recipient than the number of feedback to give and therefore the dropdown is hidden. This is the expected behaviour. However, the name should have been displayed instead, which didn't because the javascript that does it is not included in mashup.jsp.

So, the correct approach will be to include this javascript in the jsp ;) (potential complication is if it conflicts with something else in the page).

Cc: -dam...@gmail.com edi33...@gmail.com

damithc commented 10 years ago

From edi33...@gmail.com on March 27, 2014 15:14:42

Hello,

I have been trying to insert the feedbackSubmisionsEdit.js within the mashup.jsp file.

I have observed that the jquery version is a bit old v1.8.3, so I was thinking if I should try to use a newer version of jquery v1.11 for ex. If so, we could make use of the jquery migrate plugin from the pre-1.9 version and assure that the code that is already written will work as expected.

I am still trying to get the javascript working in the mashup file.

I am thrilled to hear your answer

P.S. Sorry for the past days of absence. I have had to meet up two deadlines for school. This semester is the busiest of them all, as I have understood from my senior colleagues :-)

damithc commented 10 years ago

From dam...@gmail.com on March 27, 2014 17:01:09

Edi, thanks for bringing up the jquery version issue. Let's do it as a separate issue. You can create a new issue for it. Each issue should do only one thing, in case we have to backout that patch for some reason.

damithc commented 10 years ago

From edi33...@gmail.com on March 27, 2014 18:06:27

Hello, Mr Damith,

I have created the new issue. I am planning to start working on it from tomorrow. I will also try to fix this issue.

I will start working from tomorrow morning, because now it's 03:05 AM in Romania :-).

I will be back in a couple of hours :-)

damithc commented 10 years ago

From edi33...@gmail.com on March 30, 2014 06:23:25

Hi

It's been two days now since I am trying to include the feedbackSubmissionsEdit.js in the mashup.jsp file, but I cannot get the formatRecipientLists function to work. It won't even enter the each() function from the first select within the function ( I have tried console logging it ). I have ran out of ideas and I would appreciate some help.

Also, with the mashupTest running, if I try to get the value of participantSelect I get some warnings about how an attribute is deprecated and it will return always true. I have attached a photo, because a picture is worth a thousand words :-).

I am not sure, but maybe all of the above are caused because of the old version of jquery, so, until your response, I will start working on issue 1762 ( upgrading jquery to a newer version ) :-)

Thank you in advance, Edi

Attachment: feedbackSubmissionsDeprecated.jpg

damithc commented 10 years ago

From dam...@gmail.com on March 30, 2014 06:34:16

Let's see if anybody can offer some help. I don't have any myself. :-)

damithc commented 10 years ago

From edi33...@gmail.com on March 30, 2014 06:43:50

I will keep my fingers crossed :-)

damithc commented 10 years ago

From edi33...@gmail.com on March 30, 2014 11:24:04

I believe that I have successfully upgraded the jQuery version to v1.11, keeping the code working as before.

Link to the issue on the issue tracker: http://bit.ly/1i6951o If someone could take a look and give me some feedback :-)

All the best, Edi

damithc commented 10 years ago

From arnold.k...@gmail.com on April 09, 2014 01:55:25

Owner: edi33...@gmail.com