Skip to content
This repository was archived by the owner on Oct 6, 2023. It is now read-only.

Fixes for modal content#13

Open
kevinvess wants to merge 5 commits intoedwardr:masterfrom
kevinvess:master
Open

Fixes for modal content#13
kevinvess wants to merge 5 commits intoedwardr:masterfrom
kevinvess:master

Conversation

@kevinvess
Copy link

@kevinvess kevinvess commented Dec 2, 2017

I've included fixes for the following issues:

  • Allow full WYSIWYG post content to be displayed in staff bio
  • Ability to close the modal when clicking outside of the popup window
  • Fixed display issue with staff name by escaping the attribute
  • Staff profiles link to the modal URI with query string

The WP audio shortcode in the post content still does not function in the popup, but does work if you load the page with the staff URI query string. I believe this is because the WordPress script can't run on the post content while it's hidden.

This pull request is in response to issue #12

@kevinvess
Copy link
Author

kevinvess commented Dec 2, 2017

Also– while working on these changes and thinking through how this works, I think it might be better if you enqueue a localized script with all the staff post data once which the JS could reference to pull post information from instead of using all these data attributes on each <a> link.

It would reduce the redundancy by only having one data set to reference. Just add one data attribute on each <a> link for post ID to match against the one localized data set in JS.

For example:

Staffer_Public::enqueue_scripts()

$profiles = array(
    '123' => array(
        'id' => 123,
        'slug' => 'john-doe',
        'name' => 'John Doe',
        // etc
    ),
    '124' => array(
        'id' => 124,
        'slug' => 'john-doe',
        'name' => 'John Doe',
        // etc
    ),
);

$data = array(
    'plugin_path' => plugin_dir_url( __FILE__ ),
    'profiles' => $profiles,
);

wp_localize_script( $this->plugin_name, 'cwStaffer', $data );

public/js/staffer-scripts.js

$('a.cw-launch-staffer-modal').on('click', function(e){

    var id = $(this).attr('data-staff-id');

    var profile = cwStaffer.profiles[id];

    // etc
});

This might help clean up some redundancies and could eliminate the need for a hidden <article> tag with each <li class="staff-li"> in the HTML.

Note: this is untested code, but hopefully you get the idea.

@kevinvess
Copy link
Author

@edwardr

I'd love to see this pull request included in a new release of the plugin on wordpress.org – please let me know if you'll be able to include this soon. Thanks!

@margyly
Copy link

margyly commented Mar 30, 2018

+1 on getting this PR merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants