instructorFeedbackEdit: have a 'Done editing' button?

[damithc]

From dam...@gmail.com on March 08, 2014 15:19:17

Otherwise users may be unsure how to exit that state. The button can be a dummy button to take back to another page.

Suggested by a user.

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

[damithc]

From chao.wang92@gmail.com on March 11, 2014 04:50:47

Can I fix this issue?

[damithc]

From arnold.k...@gmail.com on March 11, 2014 19:48:50

Wang Chao, finish issue 1119 first. We will assign you something from Student List/Record afterwards :)

[damithc]

From franklin...@gmail.com on March 13, 2014 22:32:52

so this is more like a ?

From dam...@gmail.com on March 13, 2014 22:45:16

Yes, can be a link. May be better to make it look like a button?

[damithc]

From franklin...@gmail.com on March 13, 2014 22:48:04

that is just css. working on it.

[damithc]

From franklin...@gmail.com on March 14, 2014 08:11:31

ready for review https://codereview.appspot.com/75830046/

[damithc]

From franklin...@gmail.com on March 14, 2014 08:12:22

the status is changed by the admin users right? i mean I cannot edit the status/assign the issue

[damithc]

From dam...@gmail.com on March 14, 2014 08:47:58

Owner: franklin...@gmail.com
Cc: arnold.k...@gmail.com
Labels: Reviewer-Arnold

[damithc]

From franklin...@gmail.com on March 14, 2014 09:34:55

https://codereview.appspot.com/75830046/

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 16, 2014 22:33:00

The table background looks rather odd as there's nothing but the button on it. Maybe just the button?

By the way, do post screenshots when doing UI changes so reviewers can see how it looks like quickly

Status: ChangesRequested

[damithc]

From franklin...@gmail.com on March 16, 2014 22:37:31

so I will remove the bg-color and keep others the same way?

[damithc]

From arnold.k...@gmail.com on March 16, 2014 22:39:07

I think you can do this simply by removing the table (so just keep the ).

From franklin...@gmail.com on March 17, 2014 02:37:37

I am appending the screenshot here. Not sure if this is OK(if this is OK, I will change the tests accordingly)

Attachment: Untitled.png

[damithc]

From arnold.k...@gmail.com on March 17, 2014 02:58:49

yes, that is ok.

[damithc]

From dam...@gmail.com on March 17, 2014 02:59:34

What if we put it beside the 'add new question' button, but outside the box? The third row of buttons doesn't look very nice.

[damithc]

From franklin...@gmail.com on March 17, 2014 03:25:31

(I am not very good at CSS though) but the original inputTable class has some properties. I will try to align them in that way

[damithc]

From franklin...@gmail.com on March 18, 2014 19:24:47

is this okay? I put the anchor inside the addnewquestion table

Cc: dam...@gmail.com

Attachment: Untitled.png

[damithc]

From franklin...@gmail.com on March 18, 2014 23:08:28

if the previous screenshot looks OK to you, i will change the test accordingly

[damithc]

From dam...@gmail.com on March 19, 2014 00:10:59

Looks ok for me.

[damithc]

From franklin...@gmail.com on March 19, 2014 03:05:18

https://codereview.appspot.com/75830046/ changing of the tests. (one issue remains though, look at this post: https://groups.google.com/forum/#!topic/teammates-contributors/uvwkcw8v8gQ )

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 19, 2014 20:02:24

Added comments at the review site. Also, do a link test from InstructorFeedbackEditPageUiTest (no need to do a verifyHtml, just click the link and ensure it has the correct header).

Status: ChangesRequested

[damithc]

From franklin...@gmail.com on March 19, 2014 21:54:59

1. Add a param to const file?
2. verifying the href of Done Editing is not enough?

[damithc]

From arnold.k...@gmail.com on March 19, 2014 22:14:05

1. They are already in the Const file
2. No. We need to make sure we really end up in the correct page also.

[damithc]

From franklin...@gmail.com on March 19, 2014 22:36:20

what sort of consts you are referring to? just url const right?

[damithc]

From arnold.k...@gmail.com on March 19, 2014 22:45:46

There are also consts for parameter names

[damithc]

From franklin...@gmail.com on March 20, 2014 02:56:00

https://codereview.appspot.com/75830046/ 1. the redirect test may not be very good because of the existence of localhost:... i can split the string, but it will be basically the same effect

Status: ReadyForReview

[damithc]

From dam...@gmail.com on March 20, 2014 03:43:42

Link testing is done in many other *UiTest classes. Follow the same approach.

[damithc]

From franklin...@gmail.com on March 20, 2014 04:34:08

TestProperties.inst().TEAMMATES_URL. Dr Damith, you are referring to this right?

Status: ChangeRequested

[damithc]

From franklin...@gmail.com on March 20, 2014 04:34:41

https://codereview.appspot.com/75830046

Status: ReadyForReview

[damithc]

From dam...@gmail.com on March 20, 2014 04:41:46

I wasn't. But never mind. Your modification is correct :-)

[damithc]

From dam...@gmail.com on March 20, 2014 09:21:43

Issue 1735 has been merged into this issue.

[damithc]

From arnold.k...@gmail.com on March 20, 2014 20:37:05

Junchao, can you help me merge and reupload? The test clashes with your last issue's commit :P

[damithc]

From arnold.k...@gmail.com on March 20, 2014 20:37:27

Status: ChangesRequested

[damithc]

From franklin...@gmail.com on March 20, 2014 23:30:19

https://codereview.appspot.com/75830046/

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 20, 2014 23:47:08

This issue was updated by revision 49c91d1241f7 .

Status: Delivered

[damithc]

From dam...@gmail.com on March 22, 2014 05:50:54

Status: Deployed
Labels: Milestone-V4.93