Gallery: Add link to option #2164

mkaz · 2017-08-02T17:54:54Z

Allow user to select link to option for images similar to core gallery.
Add to Inspector Link To options: Attachment Page, Media File, or None.

See: #1448

codecov · 2017-08-02T21:56:09Z

Codecov Report

Merging #2164 into master will increase coverage by 0.28%.
The diff coverage is 46.15%.

@@            Coverage Diff             @@
##           master    #2164      +/-   ##
==========================================
+ Coverage   23.83%   24.12%   +0.28%     
==========================================
  Files         142      142              
  Lines        4459     4609     +150     
  Branches      756      794      +38     
==========================================
+ Hits         1063     1112      +49     
- Misses       2866     2933      +67     
- Partials      530      564      +34

Impacted Files	Coverage Δ
blocks/media-upload-button/index.js	`4.25% <0%> (ø)`	⬆️
blocks/library/gallery/gallery-image.js	`50% <42.85%> (-50%)`	⬇️
blocks/library/gallery/index.js	`25.71% <60%> (+0.71%)`	⬆️
blocks/library/code/index.js	`40% <0%> (-10%)`	⬇️
blocks/library/separator/index.js	`42.85% <0%> (-7.15%)`	⬇️
blocks/api/parser.js	`97.5% <0%> (-2.5%)`	⬇️
blocks/editable/patterns.js	`1.21% <0%> (-0.61%)`	⬇️
editor/block-mover/index.js	`0% <0%> (ø)`	⬆️
editor/sidebar/table-of-contents/index.js	`0% <0%> (ø)`	⬆️
blocks/library/embed/index.js	`48.68% <0%> (+2.7%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87e46bd...b35b2e5. Read the comment docs.

timmyc

While testing this out I hit an issue with linking to the attachment page. Steps to reproduce:

1- Add new gallery block
2- Select a few images
3- Link To "Attachment Page"
4- Save

When viewing the post on the front-end of the site, the images in the gallery are not linked. It does work for linking to "Media File"

timmyc · 2017-08-02T22:19:30Z

blocks/library/gallery/index.js

+					<SelectControl
+						label={ __( 'Link to' ) }
+						selected={ linkto }
+						onBlur={ setLinkto }


Any reason to not have onChange={ setLinkto } here? Currently, after changing the value of this element, you must click off the select drop down to trigger the onBlur before you can update the post:

Ah good point, I'll update.

timmyc · 2017-08-02T22:29:06Z

I also re-triggered the CI job that had failed, all looks green now.

mkaz · 2017-08-03T19:09:31Z

@timmyc the Attachment link is weird - it works for some of my test images but not others, trying to figure out why since the data should be accurate

timmyc · 2017-08-03T20:51:15Z

@mkaz so I thought it might have just been certain images in my library too... but I tested with the same images using the gallery flow in the current editor and the links worked okay for me there with "attachment" selected.

I didn't debug further, but thought something in the following line might be the culprit:

if ( props.linkto === 'attachment' && !! props.img.link ) {

mtias · 2017-08-04T16:20:55Z

blocks/library/gallery/gallery-image.js

+		);
+	}
+
+	if ( props.linkto === 'attachment' && !! props.img.link ) {


It'd be nice to not duplicate the markup for links and do this instead:
https://github.com/WordPress/gutenberg/blob/master/blocks/library/image/index.js#L200

…a, or None

…k out

mtias · 2017-08-04T17:27:07Z

blocks/library/gallery/index.js

@@ -71,9 +77,11 @@ registerBlockType( 'core/gallery', {

 	edit( { attributes, setAttributes, focus, className } ) {
 		const { images = [], columns = defaultColumnsNumber( attributes ), align = 'none' } = attributes;
+		const setLinkto = ( value ) => setAttributes( { linkto: value } );


tiny thing, but probably should camel case the linkTo :)

mtias · 2017-08-04T17:28:45Z

blocks/library/gallery/index.js

 		const setColumnsNumber = ( event ) => setAttributes( { columns: event.target.value } );
 		const updateAlignment = ( nextAlign ) => setAttributes( { align: nextAlign } );
 		const { imageCrop = true } = attributes;
+		const { linkto = 'none' } = attributes;


These could be moved to default attrs:

defaultAttributes: { dropCap: false, },

jasmussen · 2017-08-04T17:32:38Z

Also one niggle: I'd move "Link To" down below the "Crop Images" box:

mtias · 2017-08-04T17:33:34Z

Also maybe the label + input could all fit in one row?

mkaz · 2017-08-04T17:45:54Z

@mtias I'm just using the SelectControl component and don't see any real options on how to keep it all on the same row - any suggestion?

mtias · 2017-08-04T17:47:56Z

That's fine, we could look into the component itself in the future.

timmyc · 2017-08-04T18:50:50Z

blocks/library/gallery/gallery-image.js

@@ -5,7 +5,7 @@ export default function GalleryImage( props ) {
 		case 'media':
 			href = props.img.url;
 			break;
-		case 'attacment':
+		case 'attachment':


i always misspell that word too - funny that i didn't catch that when reviewing. 😄

* Release v1.26.0 (#2153) * Add tests for Latest-Posts Bock * Have the Automation tests Scroll the Block window to help locate Block buttons on Android * Update gutenberg reference * Update Gutenberg ref * Update Gutenberg ref * New template for release PRs This PR will add a new template for release PRs to make it easier to check all the steps needed and to standardize the release PRs. This template can then be used by using this link: `https://github.com/wordpress-mobile/gutenberg-mobile/pull/new?template=release_pull_request.md` * Update template file. * Fix: remove extra padding for post title and first `Paragraph` block (#2095) * Fix: remove extra padding for post title and first `Paragraph` block * Update Gutenberg ref * Add a new androidReplacements function to comply with Android Typography lint rules * Make sure the file gutenberg.pot exists before generating android and ios strings. * Update Gutenberg ref * Update gutenberg ref * Update gutenberg ref * Update gutenberg reference * Gutenberg update * Update Gutenberg ref * Update Gutenberg ref * Update Gutenberg ref * Update Gutenberg ref * Fix: prevent ripple effect on slider cell in BottomSheet and disable it completely on iOS (#2023) * prevent ripple effect on slider cell in BottomSheet and disable it completely on iOS * Update Gutenberg ref * Update Gutenberg ref * Accept multiple headers through OnAuthHeaderRequestedListener (#2080) * Blog layout template (#2114) * Update Gutenberg ref * Update Gutenberg ref * Update gutenberg reference * Fix failing UI tests Try scrolling in the Inserter for all platforms * Disable the failing test on iOS Co-authored-by: Matthew Kevins <[email protected]> Co-authored-by: Pinar Olguc <[email protected]> * Update gutenberg reference * Feat: Column block (#1661) * update ref to master (Columns Block) * Update gutenberg reference * Fix Latests Posts Tests by expanding the scroll to button functionality * Fix lint issue * Fix typography breakage in master To a version where the typography panel is not added to block settings. * Update GB reference. * Correct slider step value (#2119) * Update ref: Correct slider step accordingly to the platform * Update gb ref Co-authored-by: Pinar Olguc <[email protected]> * v1.26.0 * Add some missing release notes * Update Podfile.lock * Update gb ref * Update bundles Co-authored-by: Chip Snyder <[email protected]> Co-authored-by: Matthew Kevins <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Sérgio Estêvão <[email protected]> Co-authored-by: jbinda <[email protected]> Co-authored-by: Chip <[email protected]> Co-authored-by: Maxime Biais <[email protected]> Co-authored-by: Tugdual de Kerviler <[email protected]> Co-authored-by: Klymentiy Haykov <[email protected]> Co-authored-by: Matthew Kevins <[email protected]> Co-authored-by: Luke Walczak <[email protected]> * Update gb ref Co-authored-by: Chip Snyder <[email protected]> Co-authored-by: Matthew Kevins <[email protected]> Co-authored-by: Gerardo Pacheco <[email protected]> Co-authored-by: Sérgio Estêvão <[email protected]> Co-authored-by: jbinda <[email protected]> Co-authored-by: Chip <[email protected]> Co-authored-by: Maxime Biais <[email protected]> Co-authored-by: Tugdual de Kerviler <[email protected]> Co-authored-by: Klymentiy Haykov <[email protected]> Co-authored-by: Matthew Kevins <[email protected]> Co-authored-by: Luke Walczak <[email protected]>

timmyc reviewed Aug 2, 2017

View reviewed changes

timmyc mentioned this pull request Aug 2, 2017

Strip whitespace on inserter search term. #2171

Merged

mkaz force-pushed the add/gallery-linkto branch from 17a95c4 to 3e6e04f Compare August 2, 2017 22:53

mtias reviewed Aug 4, 2017

View reviewed changes

mtias added the [Feature] Blocks Overall functionality of blocks label Aug 4, 2017

mkaz added 8 commits August 4, 2017 09:43

Add Inspector option for setting linkto attribute to Attachment, Medi…

079e770

…a, or None

Update labels to match core

2fb30b5

Pass linkto attribute to GalleryImage

550ef08

Add media link if linkto set

fca483f

We need link data for linking to attachment page

4fd1e10

Add attachment link if linkto specifies so

7867cf6

Switch event to Change not Blur so can Update without needing to clic…

c4b2773

…k out

Refactor logic for displaying link to simplier switch & ternary

47172d1

mkaz force-pushed the add/gallery-linkto branch from 2b51b87 to 47172d1 Compare August 4, 2017 17:02

mtias reviewed Aug 4, 2017

View reviewed changes

Move Link To select control below image crop

60aeae8

mkaz added 2 commits August 4, 2017 11:27

Switch back to onBlur, SelectControl doesnt work with onChange

a55ccaa

Fix typo :(

7b6fe4c

timmyc reviewed Aug 4, 2017

View reviewed changes

Switch linkto to camelCase linkTo

b35b2e5

mkaz merged commit 73858e5 into master Aug 7, 2017

mkaz deleted the add/gallery-linkto branch August 7, 2017 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gallery: Add link to option #2164

Gallery: Add link to option #2164

mkaz commented Aug 2, 2017

codecov bot commented Aug 2, 2017 •

edited

Loading

timmyc left a comment

timmyc Aug 2, 2017

mkaz Aug 2, 2017

timmyc commented Aug 2, 2017

mkaz commented Aug 3, 2017

timmyc commented Aug 3, 2017

mtias Aug 4, 2017

mkaz Aug 4, 2017

mtias Aug 4, 2017

mtias Aug 4, 2017

jasmussen commented Aug 4, 2017

mtias commented Aug 4, 2017

mkaz commented Aug 4, 2017

mtias commented Aug 4, 2017

timmyc Aug 4, 2017

Gallery: Add link to option #2164

Gallery: Add link to option #2164

Conversation

mkaz commented Aug 2, 2017

codecov bot commented Aug 2, 2017 • edited Loading

Codecov Report

timmyc left a comment

Choose a reason for hiding this comment

timmyc Aug 2, 2017

Choose a reason for hiding this comment

mkaz Aug 2, 2017

Choose a reason for hiding this comment

timmyc commented Aug 2, 2017

mkaz commented Aug 3, 2017

timmyc commented Aug 3, 2017

mtias Aug 4, 2017

Choose a reason for hiding this comment

mkaz Aug 4, 2017

Choose a reason for hiding this comment

mtias Aug 4, 2017

Choose a reason for hiding this comment

mtias Aug 4, 2017

Choose a reason for hiding this comment

jasmussen commented Aug 4, 2017

mtias commented Aug 4, 2017

mkaz commented Aug 4, 2017

mtias commented Aug 4, 2017

timmyc Aug 4, 2017

Choose a reason for hiding this comment

codecov bot commented Aug 2, 2017 •

edited

Loading