Mattermost-server: Allow in-line markdown images to open preview window

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

Repro:

1. Post a markdown in-line image.
2. Click on the image in the post

Example

![test image](https://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png)

Observed: Nothing

Expected: Similar to image link previews:

1. Drop shadow hover effect when over the in-line image
2. Mouse pointer displays as hand when hovering
3. Clicking opens the image in the preview window

Note: mobile app already does the expected behaviour

[bradjcoughlin]

How can I determine if this is for the web app or mobile?

[jasonblais]

Good question @bradjcoughlin - this ticket is for the webapp as the mobile app already does the expected behaviour today.

[jasonblais]

Would you be open to helping with this improvement?

[bradjcoughlin]

@jasonblais In fact, I am!

[jasonblais]

Awesome, thank you! Let us know if you have any questions :)

[hanzei]

@bradjcoughlin How is your work on this one going? Do you have any questions?

[bradjcoughlin]

@hanzei I'm planning to work on this in the next 1-2 weeks.

[jwilander]

Opening this back up as not sure when we'll be able to get to this

[abdusabri]

Hi!

I'm interested to help with this one.

I've a working proof of concept/draft implementation 🙂 (screenshots below)

As I'm new to the code base, I would probably need to ask a couple or so questions to know where/how best to implement some of the needed changes.

[jasonblais]

This is exciting, thanks @abdusabri! Happy to help if you have questions. You can also join our public channels referenced in the ticket description for help

[abdusabri]

Hi!

If I understand correctly, there would be a separate GH issue for E2E testing, right?

I'm finishing up a couple of unit tests, and should open a PR in a day or two 🙂

[jasonblais]

@abdusabri Good question. E2E tests for webapp should be added where appropriate. You can find a quick contributions checklist, which might be a helpful reference point: https://developers.mattermost.com/contribute/getting-started/contribution-checklist/

[abdusabri]

Thanks @jasonblais for the clarification! Will work on E2E tests as well.

After I'd gone through the checklist and then https://developers.mattermost.com/contribute/webapp/end-to-end-tests/, section Writing Specs, point 4. was what confused me a bit. 🙂

[jasonblais]

Let us know how we can clarify - is it regarding the core staff Release Testing spreadsheet?

[abdusabri]

Yes, this is what confused me a bit. Specially, stating that the tests should be written in the spec file after the keys in the spreadsheet, and then mentioning that This information is stated in Github’s help-wanted ticket and Jira ticket.

Maybe adding a note that this point doesn't apply to non-Area/E2E Tests tickets and contributors should add E2E tests as/where appropriate will make it extra clear. Still the referenced PR in point 5. would be very relevant, specially as its tests are written in the spec without actually referring to test keys like M14014.

Anyway, you've great documentation, and it is a joy to contribute to your projects! 🙂

[jasonblais]

@saturninoabril @lindalumitchell Heads up on above feedback with an opportunity to clarify our documentation -- specifically points 4 and 5 here: https://developers.mattermost.com/contribute/webapp/end-to-end-tests/#writing-specs

[saturninoabril]

@abdusabri @jasonblais Thank you for pointing out. I'll update the E2E documentation accordingly.

[saturninoabril]

PR submitted to improve the docs - https://github.com/mattermost/mattermost-developer-documentation/pull/359. Let me know if that sounds good or if there's any comment.

[abdusabri]

While working on E2E tests, I noticed an edge case that I would like to get a confirmation on.

It is case (C) in the figure below.

If the markdown message has the image as a link, like Mattermost/platform build status: [![Build Status](https://travis-ci.org/mattermost/platform.svg?branch=master)](https://travis-ci.org/mattermost/platform), in this case, the image already has a click action (the <a> tag), and the mouse cursor is a pointer. Implementing it similar to case (B) would result in 2 click actions happening at the same time (opening the link and the preview window).

Should I keep it as illustrated in the image (without preview)? or there should be a different behavior (like preventing the default link action for instance)?

Thanks!

[abdusabri]

Adding a screen capture to illustrate better

[jasonblais]

@abdusabri Good question, I'll ping @esethna for his thoughts

[esethna]

@abdusabri good question and good catch on finding this corner case. I would imagine that it should behave as it does now, so the link is opened but not the image preview as in case (b).

[abdusabri]

Thanks @jasonblais and @esethna for the feedback. Will wrap it up accordingly 🙂

[abdusabri]

PR ready for review 🎉 🙂

[jasonblais]

Sweet! Will queue for review next

[hughes]

Is this being implemented in a way that allows in-line markdown images to respect the "Default appearance of image previews" option? Currently they ignore the "collapsed" preference.

[jasonblais]

@esethna / @abdusabri would you know the answer to the above?

[abdusabri]

@jasonblais @hughes, as far as i understand, no. I didn't do special handling to support this. The screen capture above (from 10 days ago), sums up the implementation

[hughes]

I ask because I ended up here from this giphy plugin issue. It seems that @moussetc originally raised this issue to handle the minimization feature. I think that got interpreted as "treat inline images identically to linked images" which then got interpreted as "all images should open in a preview window".

We might just need a separate issue for the collapsible images.