Student Profile: add a 'Upload/Edit Photo' tooltip #8589 #8592
Conversation
|
@labirinto You do not need to close the PR when you have a build failure. We can make changes and push to this branch only. read our Process document. |
|
Ready for review |
| @@ -433,7 +433,8 @@ private Const() { | |||
| public static final String FEEDBACK_QUESTION_RUBRIC_ASSIGN_WEIGHTS = | |||
| "Assign weights to the columns for calculating statistics."; | |||
|
|
|||
| public static final String STUDENT_PROFILE_PICTURE = "Upload a profile picture"; | |||
| public static final String STUDENT_PROFILE_PICTURE = "Upload a profile picture. Max size 5MB. " | |||
| + ".jpg or .png extensions"; | |||
There was a problem hiding this comment.
@labirinto Is there any special reason for string concatenation of two constant string?
There was a problem hiding this comment.
@Progyan1997 I guess that is to avoid checkstyle error (Line should have a max of 125 characters)
There was a problem hiding this comment.
@Progyan1997, yes it was because of checkstyle error. (Good guess @sukanta-27:))
There was a problem hiding this comment.
public static final String STUDENT_PROFILE_PICTURE =
"Upload a profile picture. Max size 5MB. .jpg or .png extensions";This would look better though and satisfy checkstyle as well
There was a problem hiding this comment.
But the = should be on the previous line though. Otherwise it will be another checkstyle error.
There was a problem hiding this comment.
@sukanta-27 haha thanks for pointing out. I edited the next moment after posting. :P
There was a problem hiding this comment.
@tshradheya I guess I responded too quick. 😆
wkurniawan07
added
s.ToReview
| @@ -29,7 +29,12 @@ | |||
| type="button" | |||
| data-toggle="modal" | |||
| data-target="#studentPhotoUploader"> | |||
| Upload/Edit Photo | |||
| <div class="" | |||
There was a problem hiding this comment.
divin abuttonis not semantically correct. Usespaninstead.- Remove the
classattribute instead of having one with empty value.
| <div class="" | ||
| data-toggle="tooltip" | ||
| data-placement="top" | ||
| title="Upload a profile picture. Max size 5MB. .jpg or .png extensions."> |
There was a problem hiding this comment.
Re-use the tooltip in Const instead of repeating.
| @@ -433,7 +433,8 @@ private Const() { | |||
| public static final String FEEDBACK_QUESTION_RUBRIC_ASSIGN_WEIGHTS = | |||
| "Assign weights to the columns for calculating statistics."; | |||
|
|
|||
| public static final String STUDENT_PROFILE_PICTURE = "Upload a profile picture"; | |||
| public static final String STUDENT_PROFILE_PICTURE = "Upload a profile picture. Max size 5MB. " | |||
There was a problem hiding this comment.
Tweak the message a bit:
Upload a profile picture (.jpg or .png, max size 5 MB)
whipermr5
added
s.Ongoing
|
@labirinto Closing due to inactivity for a long period. Feel free to request for reopen if you're ready to resume the work here. |
Fixes #8589