openlab-at-city-tech / webworkqa

WeBWorK integration for WordPress and BuddyPress
GNU General Public License v2.0
4 stars 2 forks source link

File upload for question/answer fields #107

Closed boonebgorges closed 6 years ago

boonebgorges commented 6 years ago

For images and other supporting documents.

boonebgorges commented 6 years ago

A first pass at this functionality is available on openlabdev.org/webwork-playground. I had to make a couple design/workflow decisions, all of which are subject to revision by the group. Screencast: https://recordit.co/scbydfBvAo

  1. I opted to integrate with the WP media dialog rather than building something different. This has a couple advantages. It's familiar to users; it gets us automatic mobile device compatibility (for the most part); it allows users to attach items that they've previously attached; etc. The downside is that it's maybe a bit overkill for wanting to upload simple files.

  2. "attachment" support can take a couple forms. One is a "list" of attached items, usually displayed outside of the post content. The other is "inline" items, which are shown right in the post content. I opted for inline display. This feels like it makes more sense for images. There are also some technical reasons why it's easier to have inline items (like the reuse of uploaded images). This also avoids the design decisions required if you have a list outside the post content: where does it go, what does it look like, can you reorder items, can you remove items, etc. In 'Edit' mode, the inline items are shown as pseudo-shortcodes of the form [attachment id="12345"], which can be moved around or deleted like normal text in a way that I think is pretty easy to understand.

  3. To invoke the uploader, you click 'Add Files' just next to 'Preview'. I also added a little icon. This is totally arbitrary and perhaps not the best option. Happy to consider other solutions. Because this is very tentative, I haven't done much work to ensure that this button looks good in mobile contexts. We'll cross that bridge once we have a design decision.

Hopefully folks will have a chance to play with this a bit - or at least watch the (very short!) screencast linked above - before our meeting on May 1. cc @jennaspevack @drdrew42 who haven't accepted their invitations to this repo so cannot be added as watchers :-D

boonebgorges commented 6 years ago

A brief follow-up that I have, for the time being, limited upload formats to image files. Non-image files cannot be displayed inline; they'd have to be converted to links. Since other types of uploads probably aren't super important for v1, I've put the image-only limitation in place. We can revisit this.

drdrew42 commented 6 years ago

recordit.co link is not working for me. I'll try to play around with the playground site and see what I can accomplish...

drdrew42 commented 6 years ago

Having issues with drag-n-drop on the playground. Saving a file to my computer then uploading via "select files" worked fine though.

boonebgorges commented 6 years ago

recordit.co appears to be down. You get what you pay for.

I didn't even think to try the drag-and-drop, as it comes directly from WP. Can you test to see whether it works as expected at Dashboard > Media > Add New?

drdrew42 commented 6 years ago

It doesn't seem to work there either. Safari 11.0.1 on macOS 10.13.1

boonebgorges commented 6 years ago

Hm, drag-and-drop works for me Safari 11.0.2 OSX 10.13.2. After our call next week, we'll talk through the process of browser testing etc.

jennaspevack commented 6 years ago

@boonebgorges I will provide a mockup to adjust typography shortly, but wanted provide some feedback. Here's what I tried:

  1. Clicked on upload image in comment.
  2. Selected an image.
  3. Tried to edit an image (which opened a new Dashboard window - we might want to think about that).
  4. Tried to submit without adding alt (clicking insert multiple times), which repeated returned the alert.
  5. Entered some alt text and pressed Insert -- nothing happened.
  6. Press Insert many times -- nothing happened.
  7. Closed window and saw that short code had been added many times.

Here are a couple screenshots: fireshot capture 15 - webwork playground just another city_ - http___openlabdev org_webwork-play fireshot capture 16 - webwork playground just another city_ - http___openlabdev org_webwork-play

boonebgorges commented 6 years ago

Thanks for the detailed report, @jennaspevack. There were a couple issues. First, I'd forgotten to ensure that non-admins can add alt text to all their images - this was causing a number of the breakages (like the fact that the modal didn't close after clicking Insert, which is what led to the multiple entries in the textbox when you manually closed it). I believe I've now fixed this. I also addressed the duplicate 'you must supply alt text' warnings.

For the moment, I'm going to hide the Edit Image button, as I don't think it's very useful in this context, and you're right that it'll be confusing for users.

jennaspevack commented 6 years ago

@boonebgorges, I'm still seeing the multiple 'you must add alt text' warnings, but the multiple entries seems to be fixed.

Here is a mockup with visual adjustments to the Add Images link (See item 2.) I think it would be confusing to use Files if it really is only limited to images, but maybe @drdrew42 can confirm that. Item 4. in this mockup is also relevant to this area. https://www.dropbox.com/s/e8i2ricwn16o7xt/OLWW_LAYOUT_PROBLEMPOSTV6.png?dl=0

NOTE: There is other info in this mockup, sorry about that -- in the interest of time, I'm just working from one file. :)

boonebgorges commented 6 years ago

Thank you very much @jennaspevack !

I've made the styling changes. These changes also included the Edit icon, since it's defined in the same place as the Preview icon. (I've ignored the other stuff in the mockup.)

I can't reproduce the multiple warnings. I'm guessing you've got a sticky browser cache. Try in an incognito window, and if it's still happening, please give exact steps to duplicate. Maybe there's some specific combination of adding images and clicking around that I'm not accounting for.

boonebgorges commented 6 years ago

Screenshots:

screenshot_2018-05-07_16-54-41 screenshot_2018-05-07_16-54-29

bree-z commented 6 years ago

Hi Boone,

Here's what I've found after some testing:

attachfilestext

afterinsertclicked

multipleimages

Thanks!

boonebgorges commented 6 years ago

@bree-z Thanks. The double entry thing is because you're clicking 'Insert' more than once, which you're likely doing because the window isn't closing, which is likely happening because of some JS error. Can you do the following please:

  1. Hard-refresh your browser, just to be sure you've got the latest JS and CSS
  2. Open the JS console in your browser
  3. Run through this process again, and note what errors appear in the console
bree-z commented 6 years ago

Thanks, Boone. I did a hard refresh, and here's what I found:

console consoleff
boonebgorges commented 6 years ago

Perfect, those steps are just what I needed. I'll have a look. Thanks!

boonebgorges commented 6 years ago

Thanks again for the clear steps to reproduce, @bree-z - I've just pushed up a fix for this.

bree-z commented 6 years ago

Thanks, Boone! This looks good! The issue with the window closing is fixed, as well as the upload files styling issues.

boonebgorges commented 6 years ago

Awesome - let's close and handle more specific issues as they come up.