Skip to content
This repository has been archived by the owner on Apr 2, 2020. It is now read-only.

[WIP] Add checkbox field to use the attachment's alt or caption values #78

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

Conversation

montchr
Copy link
Contributor

@montchr montchr commented Sep 7, 2017

#77

The alt and caption attributes of the shortcode will be used by
default.

The `alt` and `caption` attributes of the shortcode will be used by
default.
@montchr
Copy link
Contributor Author

montchr commented Sep 7, 2017

I wasn't able to effectively disable the text input fields associated with the alt and caption checkboxes. I got that working for changed values, but not for the initial load of the modal. Here's what I have so far in assets/js/image-shortcake-admin.js:

		attachmentAlt: function( changed, collection, shortcode ) {
			var $altField = sui.views.editAttributeField.getField( collection, 'alt' ).$el.find('input');

			if ( changed.value === true ) {
				$altField.attr('disabled', 'disabled');
			} else {
				$altField.removeAttr('disabled');
			}
		},

		attachmentCaption: function( changed, collection, shortcode ) {
			var $captionField = sui.views.editAttributeField.getField( collection, 'caption' ).$el.find('input');

			if ( changed.value === true ) {
				$captionField.attr('disabled', 'disabled');
			} else {
				$captionField.removeAttr('disabled');
			}
		}
	}

}

/**
 * If using a recent enough version of Shortcake (0.4.0 and up),
 * attach these listeners to the attributes.
 *
 */
if ( typeof wp.shortcake !== 'undefined' && typeof wp.shortcake.hooks !== 'undefined' ) {

	wp.shortcake.hooks.addAction( 'img.attachment',             ImageShortcake.listeners.attachment        );
	wp.shortcake.hooks.addAction( 'img.linkto',                 ImageShortcake.listeners.linkto            );
	wp.shortcake.hooks.addAction( 'img.attachment_alt',         ImageShortcake.listeners.attachmentAlt     );
	wp.shortcake.hooks.addAction( 'img.attachment_caption',     ImageShortcake.listeners.attachmentCaption );

}

I think the wording of the checkbox label is clear enough to indicate what's going on without disabling the associated text input, but I'd appreciate any pointers on how I'd set the disabled attribute initially so it matches the state of the checkbox as described in the above conditionals.

@montchr
Copy link
Contributor Author

montchr commented Sep 7, 2017

Also it looks like the build errors have nothing to do with my changes.

@montchr montchr changed the title Add checkbox field to use the attachment's alt or caption values [WIP] Add checkbox field to use the attachment's alt or caption values Sep 7, 2017
@montchr
Copy link
Contributor Author

montchr commented Sep 7, 2017

I'm also unsure how to set a default value on a checkbox with Shortcake – I've tried value, default, and placeholder, none of which have the intended effect. value looks like it works until you change the value, save, and then re-open the modal to find the value has been reset.

goldenapples added a commit that referenced this pull request Sep 7, 2017
Travis no longer supports testing against pre-installed PHP5.3, and we
don't have to either. Updating the build matric so new tests can pass
(see #78 (comment))
@goldenapples
Copy link
Contributor

I wasn't able to effectively disable the text input fields associated with the alt and caption checkboxes. I got that working for changed values, but not for the initial load of the modal.

Looks like the problem is that you're checking === true. Because the value is being parsed from the shortcode string, the value of a checked state is the string "true" on the initial modal open. Can you do a looser type checking, or add some handling for this special case?

I'm also unsure how to set a default value on a checkbox with Shortcake – I've tried value, default, and placeholder, none of which have the intended effect. value looks like it works until you change the value, save, and then re-open the modal to find the value has been reset.

Hmm. I just looked into that, and it doesn't look like it's currently possible. This is a bit of a conundrum with the current architecture, since most of our field templates are based on the assumption that an empty value will be set to the default. I wonder if you could make use of the render_new hook for this?

@goldenapples
Copy link
Contributor

Also, the build issues you have should be fixed once #80 is merged in.

@montchr
Copy link
Contributor Author

montchr commented Sep 8, 2017

Can you do a looser type checking, or add some handling for this special case?

Thanks, that worked, see 2b6d625 .

Hmm. I just looked into that, and it doesn't look like it's currently possible. This is a bit of a conundrum with the current architecture, since most of our field templates are based on the assumption that an empty value will be set to the default. I wonder if you could make use of the render_new hook for this?

I tried using render_new and it appears that hook doesn't fire when editing an image shortcode. It only fires upon opening a shortcode directly from within the shortcode UI:

Note that the error that shows in the console is #75.

Another alternative solution that would allow for setting a default might be using a select field instead of a checkbox?

@goldenapples
Copy link
Contributor

I tried using render_new and it appears that hook doesn't fire when editing an image shortcode

Oh yeah. I was thinking you only needed this behavior to implement the default value, which you would only need to set on inserting a new shortcode. If you want it to fire on editing an existing post element as well, the hook to use there is render_edit, I believe.

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