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

Added option to load and enable select2 in ticket registration form #212

Open
vedanshujain wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added option to load and enable select2 in ticket registration form #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca31be7
Added option to load and enable select2 in ticket registration form
vedanshujain May 28, 2018
e9542b0
Cosmetic changes + also added select2 for "Summarize by" field.
vedanshujain Jun 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,5 +578,16 @@ window.camptix = window.camptix || { models: {}, views: {}, collections: {} };
);
} );

/**
* Select2 for dropdowns rendered by tix page
*/
$( document ).ready( function() {
if( ! $.fn.hasOwnProperty( 'select2' ) ){
return;
}
$( '.tix-setup-form select' ).select2();
$( 'select[name=tix_summarize_by]' ).select2();
});

});
}(jQuery));
8 changes: 7 additions & 1 deletion camptix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ var docCookies={getItem:function(e){return decodeURIComponent(document.cookie.re
}
};

$( document ).ready( lazyLoad.init )
$( document ).ready( lazyLoad.init );

$( document ).ready( function loadSelect2() {
if( ! tix.length || ! $.fn.hasOwnProperty( 'select2' ) )
return;
$( '#tix .tix-select2-enabled select' ).select2();
})

}(jQuery));
32 changes: 28 additions & 4 deletions camptix.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,18 @@ function enqueue_scripts() {
'enterEmail' => __( 'Please enter the e-mail addresses in the forms above.', 'camptix' ),
'ajaxURL' => admin_url( 'admin-ajax.php' ),
) );

// Let's play by the rules and print this in the <head> section.
wp_enqueue_style( 'camptix' );


if ( (bool) $this->options['select2_enabled'] ) {
Copy link
Copy Markdown
Contributor

@iandunn iandunn May 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need an option for this; adding it doesn't break backwards-compatibiliity, and I don't imagine many users wanting to disable it. Here's some more info on how we typically approach options:

https://nacin.com/2011/12/18/in-open-source-learn-to-decide/
https://wordpress.org/about/philosophy/

Copy link
Copy Markdown
Contributor Author

@vedanshujain vedanshujain May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want a configuration because of accessibility (select2/select2#3744) . Looks like this is also the blocker for merging select2 in core. What do you think ? At the very least, we should enable by default.

Copy link
Copy Markdown
Contributor

@iandunn iandunn May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. They could always call wp_dequeue_script() from a plugin to disable it, but I realize that'd require some development experience.

It sounds like one our coworkers has already forked Select2 to make it accessible, so maybe we should switch to that instead?

If you feel like it, it might even be good to send a PR to selectWoo to merge in the upstream changes. That'd help us by fixing any bugs that haven't made it downstream; help selectWoo users for the same reason; and eventually help WP Core and Drupal, by making it easy to merge the a11y fixes upstream.

In general we try to be good open source citizens, but of course it always has to be balanced against time spent on our own priorities. This seems like a really good opportunity, though.

Copy link
Copy Markdown
Contributor

@iandunn iandunn May 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, rather than trying to merge downstream, maybe it'd be better to just cherry-pick commits from downstream and send PRs upstream? That may be more efficient, since any changes will need to be tested in either direction. Whichever you think is best :)

Copy link
Copy Markdown
Contributor Author

@vedanshujain vedanshujain May 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have send in some PRs in wooSelect (woocommerce/selectWoo#18 and woocommerce/selectWoo#17) but it will take a while before they get merged. I want to wait till they get before we use wooSelect, since it doesn't have downstream changes, but we can use it right away as well. What do you think ?

Copy link
Copy Markdown
Contributor

@iandunn iandunn Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like they'll be looking at it soon, so waiting is probably fine in this case. If it takes them longer than expected, then we can always go ahead and use the setup the latest master build now, and then update it after your PRs are merged.

wp_register_style( 'select2', plugins_url( '/external/select2/css/select2.min.css', __FILE__ ), array(), $this->version );
Copy link
Copy Markdown
Contributor

@coreymckrill coreymckrill Jun 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version numbers used to register the stylesheet and script should be the version of Select2, not CampTix.

wp_register_script( 'select2', plugins_url( '/external/select2/js/select2.min.js', __FILE__ ), array(), $this->version );

wp_enqueue_style( 'select2' );
Copy link
Copy Markdown
Contributor

@coreymckrill coreymckrill Jun 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting here at that since the Select2 assets will always be both registered and enqueued if the option is enabled, you can save a step by just calling the enqueue functions with all the same parameters, and they will take care of the registration as well.

But I guess another question is, should the assets always be enqueued, or only for certain views?

wp_enqueue_script( 'select2' );
}

}

function admin_enqueue_scripts() {
Expand Down Expand Up @@ -1493,7 +1502,10 @@ function menu_setup_controls() {
$this->add_settings_field_helper( 'currency', __( 'Currency', 'camptix' ), 'field_currency' );

$this->add_settings_field_helper( 'refunds_enabled', __( 'Enable Refunds', 'camptix' ), 'field_enable_refunds', false,
__( "This will allows your customers to refund their tickets purchase by filling out a simple refund form.", 'camptix' )
__( "This will allow your customers to refund their tickets purchase by filling out a simple refund form.", 'camptix' )
);
$this->add_settings_field_helper( 'select2_enabled', __( 'Enable Select2', 'camptix' ), 'field_enable_select2', false,
__( "This will render all the dropdown answers as a searchable dropdown instead of normal select dropdown", 'camptix' )
);

break;
Expand Down Expand Up @@ -1621,7 +1633,7 @@ function validate_options( $input ) {
if ( isset( $input['refunds_date_end'], $input['refunds_enabled'] ) && (bool) $input['refunds_enabled'] && strtotime( $input['refunds_date_end'] ) )
$output['refunds_date_end'] = $input['refunds_date_end'];

$yesno_fields = array( 'refunds_enabled' );
$yesno_fields = array( 'refunds_enabled', 'select2_enabled' );

// Beta features checkboxes
if ( $this->beta_features_enabled )
Expand Down Expand Up @@ -1766,6 +1778,17 @@ function field_enable_refunds( $args ) {
<?php
}

function field_enable_select2( $args ) {?>
<div id="tix-select2-enabled-radios">
<label class="tix-yes-no description"><input type="radio" name="<?php echo esc_attr( $args['name'] ); ?>" value="1" <?php checked( $args['value'], true ); ?>> <?php _e( 'Yes', 'camptix' ); ?></label>
<label class="tix-yes-no description"><input type="radio" name="<?php echo esc_attr( $args['name'] ); ?>" value="0" <?php checked( $args['value'], false ); ?>> <?php _e( 'No', 'camptix' ); ?></label>
</div>
<?php if ( isset( $args['description'] ) ) : ?>
<p class="description"><?php echo wp_kses_post( $args['description'] ); ?></p>
<?php endif; ?>
<?php
}

/**
* The currency field for the Settings API.
*/
Expand Down Expand Up @@ -5480,7 +5503,8 @@ function form_attendee_info() {
$questions = $this->get_sorted_questions( $ticket->ID );
?>
<input type="hidden" name="tix_attendee_info[<?php echo esc_attr( $i ); ?>][ticket_id]" value="<?php echo intval( $ticket->ID ); ?>" />
<table class="tix_tickets_table tix-attendee-form">
<table class="tix_tickets_table tix-attendee-form <?php echo ( isset($this->options['select2_enabled']) && (bool) ( $this->options['select2_enabled'] ) ) ?
Copy link
Copy Markdown
Contributor

@iandunn iandunn May 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the places we'd like to add it. It'd also be use it for the currency dropdown and the Summarize fields in wp-admin.

Copy link
Copy Markdown
Contributor Author

@vedanshujain vedanshujain May 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a separate patch for currency, since its not a part of camptix. Or am I confusing with a different currency dropdown ?

Copy link
Copy Markdown
Contributor

@iandunn iandunn Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. CampTix has a currency dropdown on the Tickets > Setup > General screen. The Summarize one is under Tickets > Tools.

Copy link
Copy Markdown
Contributor Author

@vedanshujain vedanshujain Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. Added.

'tix-select2-enabled' : '' ?>">
<tbody>
<tr>
<th colspan="2">
Expand Down
Loading