Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Slideshows: If gallery has single image, slideshow is not displayed #20

Closed
richardmtl opened this issue Dec 19, 2013 · 18 comments · Fixed by #1046
Closed

Slideshows: If gallery has single image, slideshow is not displayed #20

richardmtl opened this issue Dec 19, 2013 · 18 comments · Fixed by #1046
Assignees
@lezama
Labels
[Feature] Shortcodes / Embeds [Type] Bug When a feature is broken and / or not performing as intended

Comments

@richardmtl
Copy link
Contributor

If you create a slideshow with a single image, the image appears but without the regular slideshow chrome, so it looks small and abandonded

image

If you change the gallery type to Squares or Circles, the resulting single image gallery shows up much larger and styled and looks better, than a single image slideshow.
@georgestephanis
Copy link
Member

Can we disable slideshows as an option unless they've selected two or more images? Or is it a shortcode and not done through the media modal?

@richardmtl
Copy link
Contributor Author

A slideshow can be created both through the shortcode or through the media modal; because of the shortcode, I think we'll need to prettify the single-image "gallery" output.

@georgestephanis
Copy link
Member

Or just use the shortcode_atts_{$shortcode} filter detailed by Mark Jaquith here to change the format away from Slideshow, defaulting it to something else, and let internals take over?

We only support current and current - 1 for core, so it only being introduced in 3.6 isn't an issue.

@richardmtl
Copy link
Contributor Author

I get conceptually what you mean, but coding it is a bit beyond me. I approve of your idea, however! One question: what would the result of "let internals take over" be? Displaying a single image as if it were an inserted image, without any of the slodeshow chrome (or missing chrome, as it were)? I'd be fine with that.

@blobaugh
Copy link
Contributor

@georgestephanis did you have more ideas for @richardmtl ?

@ghost ghost assigned lezama Jan 23, 2014
@lezama
Copy link
Contributor

lezama commented Jan 23, 2014

This is the line in question: https://github.com/Automattic/jetpack/blame/master/modules/shortcodes/slideshow.php#L139

@cathyjf
Copy link
Contributor

cathyjf commented Jan 23, 2014

I've been looking at this. It turns out that removing that special check causes problems because the slideshow JavaScript is broken with < 2 images. There is also a related bug in the slideshow widget code.

@lezama
Copy link
Contributor

lezama commented Jan 23, 2014

I have created slideshow-one-image branch with a patch that should fix it.

36e3152

@cathyjf @blobaugh could you review this?

There is also a related bug in the slideshow widget code.

@cathyjf what it is?

@cathyjf
Copy link
Contributor

cathyjf commented Jan 23, 2014

@lezama, the related bug is that the widget has no special check, so with zero images it just displays a perpetual loading sign. I'll add a commit to your branch that fixes that.

@cathyjf
Copy link
Contributor

cathyjf commented Jan 23, 2014

@lezama, I did that in 3eeb80c in the slideshow-one-image branch

@lezama
Copy link
Contributor

lezama commented Jan 23, 2014

@cathyjf perfect, thanks!

@georgestephanis
Copy link
Member

Can this be merged? If so, delay it for 3.1 -- since we're in beta now.

@georgestephanis georgestephanis added this to the 3.1 Freeze milestone Apr 30, 2014
@cathyjf
Copy link
Contributor

cathyjf commented Apr 30, 2014

I tested this a while ago and it's still a bit buggy, but it's not worse than the current version.

@blobaugh
Copy link
Contributor

Looks to be working fine in my userland tests. Lets merge :)

@richardmtl
Copy link
Contributor Author

Yay!

@georgestephanis
Copy link
Member

Derp, mis-read. Merging.

@georgestephanis
Copy link
Member

Wait, I can't read right now. Ignore me, my brain's fried.

@blobaugh blobaugh modified the milestones: 3.2, 3.1 Freeze Jul 8, 2014
@blobaugh blobaugh mentioned this issue Aug 27, 2014
Merged
@blobaugh
Copy link
Contributor

Closed in favor of #1046

@blobaugh blobaugh removed this from the 3.2 milestone Aug 27, 2014
kraftbj pushed a commit that referenced this issue Jan 12, 2021
@jeherve
Avoid errors when the plugin is installed manually.
1d5a893 
Fixes #20

When installing the plugin via the zip file generated from GitHub,
the directory name is jetpack-beta-master. We should use that naming convention in the plugin as well.
jeherve added a commit that referenced this issue Jun 18, 2021
@jeherve
Fix Travis configuration (#20)
a374ffb 
Travis should now send Slack notifications every time a build fails
@knutsp knutsp mentioned this issue Apr 6, 2022
Open
leogermani pushed a commit that referenced this issue Aug 12, 2022
@kraftbj
Merge pull request #20 from iamwebrocker/fix-1and1-rewritecond
b0c0f45 
Adjusts the 1&1 workaround to be more specific to the hosting platform to avoid false positives with other providers. Fixes #19
@cliffordp cliffordp mentioned this issue Oct 20, 2023
Closed
@gogdzl gogdzl mentioned this issue May 13, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lezama lezama

Labels
[Feature] Shortcodes / Embeds [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Slideshow one image Automattic/jetpack
5 participants
@lezama @cathyjf @blobaugh @georgestephanis @richardmtl