TEAMMATES / teammates

This is the project website for the TEAMMATES feedback management tool for education
https://teammatesv4.appspot.com/
GNU General Public License v2.0
1.67k stars 3.3k forks source link

Instructor Course Enroll page: Improve UI for enrollment table #9530

Closed monmanuela closed 5 years ago

monmanuela commented 5 years ago

Steps to reproduce

  1. Go to /web/instructor/courses
  2. Create a new course if there isn't any
  3. Click the Enroll button
  4. Put in some details of students to be enrolled (can copy paste from the example under More info)
  5. Enrollment Results table will appear image

Expected behaviour

The styling should look like this: image

pbrahmbhatt3 commented 5 years ago

can I work on this?

monmanuela commented 5 years ago

Hi @pbrahmbhatt3 , go ahead! Do refer to the docs and ask if you have any questions 😄

Aarabhi2017 commented 5 years ago

Is this still being worked on?

monmanuela commented 5 years ago

@Aarabhi2017 Since the previous person seems inactive, feel free to work on this :)

amrut-prabhu commented 5 years ago

@monmanuela In the scope of this issue's title, the enrollment table Existing Students can also be improved. Some other aspects of the page should be changed too.

I think we need to create new issues for these, but do you think some of these can be added in this firstTimers issue?

monmanuela commented 5 years ago

@amrut-prabhu Thanks for pointing these out!

  • Existing Students card flickers when clicked if the table is empty.
  • I feel that the No existing students in course. message can be presented in a better way.
  • If there are existing students, the table should not have empty rows at the bottom (since they can't be edited anyways).
  • If student(s) are enrolled and the Existing Students has been opened, a page refresh is required to update it.

The issues mentioned above are valid concerns related to UI/UX. I checked the code though, it doesn't seem to have an easy fix. It needs more research into Handsontable, so I don't see it as a first timer issue. @tanhengyeow, git blame says that you worked on it last time, so maybe you can advise further?

  • Clicking the Enroll Students button doesn't provide any feedback to the user since the success/error message is shown at the top of the page (which may be missed unless you scroll to the top of the page)
  • The error message itself can be updated to be more user friendly.

This is part of #9346 and I'm working on it :)

tanhengyeow commented 5 years ago

Hi @amrut-prabhu

Thanks for pointing out your observations! They are valid and indeed they require some thoughts before we proceed on fixing them. Some of these issues have been pointed out in some other issues but it's good we reiterate them and keep track of them here:

We can tackle the easy ones first such as this below:

I feel that the No existing students in course. message can be presented in a better way.

Any better suggestions?

If student(s) are enrolled and the Existing Students has been opened, a page refresh is required to update it.

Refer to https://github.com/TEAMMATES/teammates/pull/9340#issuecomment-455873486

If there are existing students, the table should not have empty rows at the bottom (since they can't be edited anyways).

Reason for this is that Handsontable (HOT) requires a fixed number of rows to display. If there are only 1-3 rows (assuming there are only 1-3 students), it looks kinda odd that the HOT's height is only X pixels, compared to the enroll HOT presented directly below it.

Existing Students card flickers when clicked if the table is empty.

Valid point that I've not really looked much into. PRs/suggestions welcome :)

Clicking the Enroll Students button doesn't provide any feedback to the user since the success/error message is shown at the top of the page (which may be missed unless you scroll to the top of the page) The error message itself can be updated to be more user friendly.

For @monmanuela, you can kind of leave this section to the later part as there are plans to refactor the backend API to return enroll messages in a better way, refer #8985. Showing the current chunk of error messages in a snackbar isn't that user friendly either.

amrut-prabhu commented 5 years ago

@monmanuela @tanhengyeow Thanks for your replies.

I feel that the No existing students in course. message can be presented in a better way. Any better suggestions?

I'm no UI expert by any means, but from my experience as an end user, how about we display the message in the expanded card (like the 2nd one below)?

2019-03-24_22h03_36

pbrahmbhatt3 commented 5 years ago

Hi @monmanuela

I was trying to run the TEAMMATES on my local machine. When I try npm run start, It builds successfully but it never stops loading(I cannot see any data). I have followed the steps given in the document. Any idea on where I might be going wrong?

monmanuela commented 5 years ago

Hi @pbrahmbhatt3, seems that you're facing the same issue as this. Have you run the backend (./gradlew appengineStart)? This is because npm run start only runs the frontend :)

pbrahmbhatt3 commented 5 years ago

It works! Is this issue still open? If yes I would like to work on it.

monmanuela commented 5 years ago

@pbrahmbhatt3 Yes, go ahead. This issue only requires small changes in instructor-course-enroll-page so it shouldn't take very long :)