The preview window currently opens as a new window. There is no good reason why it shouldn't open in modal. If we do this we need to make it configurable.

CommentFileSizeAuthor
#8 2784055-preview-modal-8.patch5.31 KBbleen
#7 2784055-preview-modal-7.patch0 bytesbleen
#6 2784055-preview-modal-6.patch2.83 KBbleen
#2 optionally_open_preview-2784055-2.patch2.47 KBbleen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bleen created an issue. See original summary.

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Status: Active » Needs work
FileSize
optionally_open_preview-2784055-2.patch2.47 KB

Everything about this patch works except that when the href of the preview link is changed dynamically it doesn't seem to get registered by the ajax logic that opens the modal. Help welcome ...

see https://drupal.stackexchange.com/questions/259566/dynamically-changing-t...

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented

this is a reminder to myself to look at this stackexchange answer more carefully: https://drupal.stackexchange.com/a/229328/63043

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented

Singing else to look at: https://www.drupal.org/project/ctools/issues/2627306#comment-11888471

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Related issues: +#2764931: Contextual links don't work with 'use-ajax' links

almost certainly related: #2764931: Contextual links don't work with 'use-ajax' links

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
FileSize
2784055-preview-modal-6.patch2.83 KB

Eureka!!

This patch works. I would like to do some cleanup and I'm still deciding if this should really be optional or not.

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Title: Optionally open preview window in a modal » Open preview window in a modal
Status: Needs work » Needs review
FileSize
2784055-preview-modal-7.patch0 bytes

I think this is ready to be reviewed

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
FileSize
2784055-preview-modal-8.patch5.31 KB

Doh!!! I think _this_ is ready to be reviewed

The last submitted patch, 7: 2784055-preview-modal-7.patch, failed testing. View results

  • bleen committed 9700500 on 8.x-1.x 
    Issue #2784055 by bleen: Open preview window in a modal
bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Status: Needs review » Fixed

I never got a good review, but I think this is a good change. I leave it in dev for a while and people can report any issues

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

esod’s picture

esod CreditAttribution: esod at Memorial Sloan Kettering Cancer Center commented

Patch #8 works well for us on 8.x-1.0-beta6.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia at Acquia commented
Related issues: +#3098235: Preview Modal Conflicts with Media Library Modal

The change from a separate browser tab/window to a modal was a good step! Thanks for that.

However, an image field widget can already itself be in a modal (for example within the media library), and modals within modals is problematic. See #3098235: Preview Modal Conflicts with Media Library Modal for ideas/discussion on fixing that.

smurfxx’s picture

smurfxx CreditAttribution: smurfxx commented

Can you please convert this patch for Drupal 7 version?