[#10941] Copying questions requires scrolling the whole list of current questions #10961
Conversation
|
Hi @Jellwood, these parts of your pull request do not appear to follow our contributing guidelines:
|
Jellwood
changed the title
|
Hello thanks for contributing to teammates! Could you please record a short gif/video demonstrating your fix for the modal? |
|
Hi! I am so sorry about that, this GIF shows the process of copying a question in addition to scrolling through the table until the end to show the table formatting, and finally selecting to ensure that the functionality still works!
|
madanalogy
added
s.ToReview
There was a problem hiding this comment.
The code looks fine to me.
However, the original issue has another small checkbox fix proposal.
You can either attempt it here or modify PR description to 'Part of #10941'.
|
Sorry about that, I'll try to get it working and having the checkboxes size up |
madanalogy
added
s.Ongoing
|
I was able to get the checkbox elements to a larger size for Chrome, I tested using Chrome and Firefox, and the boxes are similar sizes in both browsers. I was able to replicate the smaller checkbox sizes in Chrome, and I used that as the baseline. I do not have access to Safari to ensure that it works in this browser, and I can build in my Windows environment this weekend as well to make sure that Internet Explorer also works with these new additions if needed. The GIF below shows my Firefox browser then my Chrome browser to show that both browsers are displaying similar sized checkboxes. |
madanalogy
added
s.ToReview
| td { | ||
| text-align: center; | ||
| vertical-align: middle; | ||
| } |
There was a problem hiding this comment.
I am not entirely sure the central alignment improves the readability. I will leave this to other reviewers' opinions.
Original:
After:
There was a problem hiding this comment.
Speaking for myself: I'd prefer the original look. That allows reading in a more natural manner.
There was a problem hiding this comment.
For UI decisions such as this one, best to check with @damithc
There was a problem hiding this comment.
Previously we have standardized that such tables with text should be left-aligned (i.e. original)
There was a problem hiding this comment.
I can make those changes and have them in tomorrow if need be. That was my bad, I forgot that the earlier demo questions tended to have shorter questions that appeared to have centered check boxes and I did not want to break that alignment in adding scaling. If the original left-aligned is the standard, I can go back and make sure everything fits that standard.
There was a problem hiding this comment.
@t-cheepeng was involved in standardizing this kind of UI issues. @t-cheepeng do we have a document of the decisions taken? If not, we should.
There was a problem hiding this comment.
@damithc Yes, we do have a document for this. But it was not made made public/posted on github
There was a problem hiding this comment.
@damithc Yes, we do have a document for this. But it was not made made public/posted on github
I see. Let's add it to our docs. At least a public link to the Google doc.
Derek-Hardy
added
s.FinalReview
Derek-Hardy
added
s.Ongoing
t-cheepeng
added
s.ToReview
madanalogy
added
c.Bug
|
Thank you for your contribution! :) |
Fixes #10941
PR Checklist
Outline of Solution
The footer was attached by moving all of the styling of the modal footer and the table in the modal window within the super CSS file for the modal window for copying. Once the footer was the appropriate width, the height was fixed to allow for a table border the same height so that the footer does not interfere with the last question. The behavior was preserved from the clip shown in the issue and the table did not change any functionalities.